From 75cdc2db4beae94318117fdbcc06525575d78719 Mon Sep 17 00:00:00 2001 From: prasanna Date: Wed, 5 Aug 2026 22:43:53 +0530 Subject: [PATCH 1/3] feat: add comprehensive conversation API coverage Signed-off-by: prasanna --- .../src/executor/accumulator.rs | 7 + .../src/executor/compaction.rs | 15 +- .../src/executor/engine.rs | 12 +- .../agentic-server-core/src/executor/mod.rs | 4 +- .../src/executor/modes/conversation.rs | 111 +++++- .../src/executor/modes/response.rs | 14 +- .../src/executor/rehydrate.rs | 22 +- .../src/executor/request.rs | 1 + .../src/executor/upstream.rs | 2 +- crates/agentic-server-core/src/lib.rs | 5 +- .../src/storage/conversation.rs | 222 +++++++++++- crates/agentic-server-core/src/storage/mod.rs | 4 +- .../src/storage/models/conversation.rs | 219 ++++++++++-- .../src/storage/models/item.rs | 205 +++++++++-- .../src/storage/models/response.rs | 56 +++- .../src/storage/response.rs | 37 +- .../src/storage/types/conversation.rs | 2 + .../src/storage/types/item.rs | 33 +- .../src/storage/types/mod.rs | 2 +- .../src/storage/types/response.rs | 3 + .../src/types/request_response.rs | 210 +++++++++++- crates/agentic-server/src/app.rs | 18 +- crates/agentic-server/src/auth.rs | 5 + crates/agentic-server/src/handler/common.rs | 7 - .../src/handler/http/conversations.rs | 317 +++++++++++++++++- crates/agentic-server/src/handler/http/mod.rs | 5 +- .../src/handler/http/responses.rs | 31 +- crates/agentic-server/src/handler/mod.rs | 5 +- .../src/handler/websocket/responses.rs | 15 +- 29 files changed, 1427 insertions(+), 162 deletions(-) diff --git a/crates/agentic-server-core/src/executor/accumulator.rs b/crates/agentic-server-core/src/executor/accumulator.rs index fca3f9cc..4ec86786 100644 --- a/crates/agentic-server-core/src/executor/accumulator.rs +++ b/crates/agentic-server-core/src/executor/accumulator.rs @@ -533,6 +533,13 @@ impl ResponseAccumulator { previous_response_id: previous_response_id.map(str::to_string), conversation_id: self.conversation_id, instructions: instructions.map(str::to_string), + parallel_tool_calls: true, + temperature: None, + tool_choice: crate::types::io::ToolChoice::Auto, + tools: Vec::new(), + top_p: None, + truncation: None, + metadata: None, } } } diff --git a/crates/agentic-server-core/src/executor/compaction.rs b/crates/agentic-server-core/src/executor/compaction.rs index 7d1eb91a..53d9d938 100644 --- a/crates/agentic-server-core/src/executor/compaction.rs +++ b/crates/agentic-server-core/src/executor/compaction.rs @@ -1,5 +1,6 @@ +#![allow(clippy::missing_errors_doc)] + use crate::executor::error::{ExecutorError, ExecutorResult}; -use crate::executor::rehydrate::rehydrate_conversation; use crate::executor::request::{ExecutionContext, RequestContext}; use crate::executor::upstream::fetch_blocking_payload; use crate::types::event::MessageStatus; @@ -199,6 +200,7 @@ pub(crate) async fn compact_items( response_id: uuid7_str("resp_"), conversation_id: None, conversation_version: None, + tenant_id: None, }; let response = fetch_blocking_payload(&ctx, exec_ctx, auth).await?; let summary = completed_summary_text(&response)?; @@ -262,6 +264,15 @@ pub async fn compact_response( request: CompactRequest, exec_ctx: &ExecutionContext, auth: Option<&str>, +) -> ExecutorResult { + compact_response_for_tenant(request, exec_ctx, auth, None).await +} + +pub async fn compact_response_for_tenant( + request: CompactRequest, + exec_ctx: &ExecutionContext, + auth: Option<&str>, + tenant_id: Option, ) -> ExecutorResult { if request.input.is_none() && request.previous_response_id.is_none() { return Err(ExecutorError::InvalidRequest( @@ -275,7 +286,7 @@ pub async fn compact_response( request.instructions, ); payload.previous_response_id = request.previous_response_id; - let mut ctx = rehydrate_conversation(payload, exec_ctx).await?; + let mut ctx = crate::executor::rehydrate::rehydrate_conversation_for_tenant(payload, exec_ctx, tenant_id).await?; let model = ctx.enriched_request.model.clone(); let instructions = ctx.enriched_request.instructions.clone(); let input = std::mem::replace(&mut ctx.enriched_request.input, ResponsesInput::Items(Vec::new())); diff --git a/crates/agentic-server-core/src/executor/engine.rs b/crates/agentic-server-core/src/executor/engine.rs index cf0d5744..a9a469d8 100644 --- a/crates/agentic-server-core/src/executor/engine.rs +++ b/crates/agentic-server-core/src/executor/engine.rs @@ -24,7 +24,6 @@ use crate::events::EventFrame; use crate::executor::error::ExecutorResult; use crate::executor::inference::DONE_MARKER; use crate::executor::persist::persist_if_needed; -use crate::executor::rehydrate::rehydrate_conversation; use crate::executor::request::{ExecutionContext, RequestContext}; use crate::executor::upstream::{emit_deferred_stream_events, fetch_blocking_payload, fetch_stream_payload}; use crate::tool::ToolRegistry; @@ -453,6 +452,7 @@ pub struct ExecuteRequest { payload: RequestPayload, exec_ctx: Arc, client_auth: Option, + tenant_id: Option, } impl ExecuteRequest { @@ -462,6 +462,7 @@ impl ExecuteRequest { payload, exec_ctx, client_auth: None, + tenant_id: None, } } @@ -472,6 +473,12 @@ impl ExecuteRequest { self } + #[must_use] + pub fn with_tenant_id(mut self, tenant_id: Option) -> Self { + self.tenant_id = tenant_id; + self + } + /// Execute one stateful conversation turn. /// /// Returns `Either::Left(ResponsePayload)` for non-streaming requests, or @@ -490,7 +497,8 @@ impl ExecuteRequest { tools = self.payload.tools.as_ref().map_or(0, Vec::len), "executor received responses request" ); - let ctx = rehydrate_conversation(self.payload, &self.exec_ctx).await?; + let ctx = + super::rehydrate::rehydrate_conversation_for_tenant(self.payload, &self.exec_ctx, self.tenant_id).await?; if ctx.original_request.stream { Ok(Either::Right(run_stream(ctx, self.exec_ctx, self.client_auth))) } else { diff --git a/crates/agentic-server-core/src/executor/mod.rs b/crates/agentic-server-core/src/executor/mod.rs index 72fca307..e8a55d6e 100644 --- a/crates/agentic-server-core/src/executor/mod.rs +++ b/crates/agentic-server-core/src/executor/mod.rs @@ -16,7 +16,7 @@ mod gateway; pub mod gateway_accumulator; mod upstream; -pub use compaction::compact_response; +pub use compaction::{compact_response, compact_response_for_tenant}; pub use engine::{BoxStream, ExecuteRequest, create_conversation, execute}; pub use error::{ExecutorError, ExecutorResult}; pub use inference::call_inference; @@ -24,6 +24,6 @@ pub use messages_loop::run_messages_loop; pub use messages_stream::run_messages_stream; pub use modes::{ConversationHandler, ResponseHandler}; pub use persist::{persist_response, persist_turn}; -pub use rehydrate::rehydrate_conversation; +pub use rehydrate::{rehydrate_conversation, rehydrate_conversation_for_tenant}; pub use request::ExecutionContext; pub use request::RequestContext; diff --git a/crates/agentic-server-core/src/executor/modes/conversation.rs b/crates/agentic-server-core/src/executor/modes/conversation.rs index e5479fab..ae87205f 100644 --- a/crates/agentic-server-core/src/executor/modes/conversation.rs +++ b/crates/agentic-server-core/src/executor/modes/conversation.rs @@ -1,7 +1,12 @@ //! Conversation storage handler — owns all conversation store operations. +#![allow(clippy::missing_errors_doc)] + +use serde_json::Value; + use crate::storage::{ - ConversationData, ConversationSnapshot, ConversationStore, InOutItem, ResponseMetadata, StorageError, + ConversationData, ConversationItemData, ConversationItemPage, ConversationSnapshot, ConversationStore, InOutItem, + ResponseMetadata, StorageError, }; use crate::types::io::OutputItem; @@ -33,7 +38,10 @@ impl ConversationHandler { .conversation_id .as_deref() .ok_or_else(|| ExecutorError::InvalidRequest("conversation_id is required for get_or_create".into()))?; - self.store.get_or_create(conv_id).await.map_err(ExecutorError::Storage) + self.store + .get_or_create_for_tenant(conv_id, ctx.tenant_id.as_deref()) + .await + .map_err(ExecutorError::Storage) } /// Gets an existing conversation. @@ -49,7 +57,17 @@ impl ConversationHandler { .conversation_id .as_deref() .ok_or_else(|| ExecutorError::InvalidRequest("conversation_id is required for get".into()))?; - self.store.get(conv_id).await.map_err(ExecutorError::Storage) + self.store + .get_for_tenant(conv_id, ctx.tenant_id.as_deref()) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn get_by_id(&self, conversation_id: &str, tenant_id: Option<&str>) -> ExecutorResult { + self.store + .get_for_tenant(conversation_id, tenant_id) + .await + .map_err(ExecutorError::Storage) } /// Creates a brand-new conversation with a freshly generated ID. @@ -60,6 +78,87 @@ impl ConversationHandler { self.store.create().await.map_err(ExecutorError::Storage) } + pub async fn create_with_items( + &self, + tenant_id: Option<&str>, + metadata: Option<&Value>, + items: Vec, + ) -> ExecutorResult { + self.store + .create_with_items_for_tenant(tenant_id, metadata, items) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn update_metadata( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + metadata: Option<&Value>, + ) -> ExecutorResult { + self.store + .update_metadata_for_tenant(conversation_id, tenant_id, metadata) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn delete(&self, conversation_id: &str, tenant_id: Option<&str>) -> ExecutorResult<()> { + self.store + .delete_for_tenant(conversation_id, tenant_id) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn append_items( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + items: Vec, + ) -> ExecutorResult> { + self.store + .append_items_for_tenant(conversation_id, tenant_id, items) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn list_items( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + after: Option<&str>, + limit: usize, + descending: bool, + ) -> ExecutorResult { + self.store + .list_items_for_tenant(conversation_id, tenant_id, after, limit, descending) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn get_item( + &self, + conversation_id: &str, + item_id: &str, + tenant_id: Option<&str>, + ) -> ExecutorResult { + self.store + .get_item_for_tenant(conversation_id, item_id, tenant_id) + .await + .map_err(ExecutorError::Storage) + } + + pub async fn delete_item( + &self, + conversation_id: &str, + item_id: &str, + tenant_id: Option<&str>, + ) -> ExecutorResult<()> { + self.store + .delete_item_for_tenant(conversation_id, item_id, tenant_id) + .await + .map_err(ExecutorError::Storage) + } + /// Loads all history items for the conversation referenced by the request. /// /// Reads `conversation_id` from `ctx.original_request`. Returns an empty vec @@ -86,7 +185,7 @@ impl ConversationHandler { .as_deref() .ok_or_else(|| ExecutorError::InvalidRequest("conversation_id is required for rehydrate".into()))?; self.store - .rehydrate_snapshot(conv_id) + .rehydrate_snapshot_for_tenant(conv_id, ctx.tenant_id.as_deref()) .await .map_err(ExecutorError::Storage) } @@ -121,8 +220,9 @@ impl ConversationHandler { new_items.extend(output_items.into_iter().map(InOutItem::Output)); self.store - .persist_if_version( + .persist_if_version_for_tenant( &conversation_id, + ctx.tenant_id.as_deref(), conversation_version, &ctx.response_id, metadata.previous_response_id.as_deref(), @@ -176,6 +276,7 @@ mod tests { response_id: "resp_test".into(), conversation_id: conversation_id.map(str::to_string), conversation_version: None, + tenant_id: None, } } diff --git a/crates/agentic-server-core/src/executor/modes/response.rs b/crates/agentic-server-core/src/executor/modes/response.rs index 842634ed..995f4c93 100644 --- a/crates/agentic-server-core/src/executor/modes/response.rs +++ b/crates/agentic-server-core/src/executor/modes/response.rs @@ -31,7 +31,10 @@ impl ResponseHandler { .previous_response_id .as_deref() .ok_or_else(|| ExecutorError::InvalidRequest("previous_response_id is required for get".into()))?; - self.store.get(prev_id).await.map_err(ExecutorError::Storage) + self.store + .get_for_tenant(prev_id, ctx.tenant_id.as_deref()) + .await + .map_err(ExecutorError::Storage) } /// Validates that the response for `previous_response_id` exists. @@ -57,7 +60,10 @@ impl ResponseHandler { let Some(prev_id) = ctx.original_request.previous_response_id.as_deref() else { return Ok(vec![]); }; - self.store.rehydrate(prev_id).await.map_err(ExecutorError::Storage) + self.store + .rehydrate_for_tenant(prev_id, ctx.tenant_id.as_deref()) + .await + .map_err(ExecutorError::Storage) } /// Persists a response record — only the new items from this turn. @@ -82,12 +88,13 @@ impl ResponseHandler { new_items.extend(output_items.into_iter().map(InOutItem::Output)); self.store - .persist_with_conversation_id( + .persist_with_conversation_id_for_tenant( &ctx.response_id, ctx.conversation_id.as_deref(), metadata.previous_response_id.as_deref(), new_items, &metadata, + ctx.tenant_id.as_deref(), ) .await .map_err(ExecutorError::Storage) @@ -132,6 +139,7 @@ mod tests { response_id: "resp_test".into(), conversation_id: None, conversation_version: None, + tenant_id: None, } } diff --git a/crates/agentic-server-core/src/executor/rehydrate.rs b/crates/agentic-server-core/src/executor/rehydrate.rs index 6b9fdf08..eb43645c 100644 --- a/crates/agentic-server-core/src/executor/rehydrate.rs +++ b/crates/agentic-server-core/src/executor/rehydrate.rs @@ -3,6 +3,8 @@ //! Builds a [`RequestContext`] by loading prior turns from storage and //! injecting them into the enriched request before it is forwarded to the LLM. +#![allow(clippy::missing_errors_doc)] + use crate::executor::error::{ExecutorError, ExecutorResult}; use crate::executor::request::{ExecutionContext, RequestContext}; use crate::storage::InOutItem; @@ -26,6 +28,14 @@ use crate::utils::uuid7_str; pub async fn rehydrate_conversation( request: RequestPayload, exec_ctx: &ExecutionContext, +) -> ExecutorResult { + rehydrate_conversation_for_tenant(request, exec_ctx, None).await +} + +pub async fn rehydrate_conversation_for_tenant( + request: RequestPayload, + exec_ctx: &ExecutionContext, + tenant_id: Option, ) -> ExecutorResult { let response_id = uuid7_str("resp_"); let new_input_items: Vec = Vec::from(&request.input); @@ -39,11 +49,12 @@ pub async fn rehydrate_conversation( response_id, conversation_id: None, conversation_version: None, + tenant_id, }; if ctx.original_request.conversation_id.is_some() && ctx.original_request.previous_response_id.is_some() { return Err(ExecutorError::InvalidRequest( - "provide only one of conversation_id or previous_response_id".into(), + "provide only one of conversation or previous_response_id".into(), )); } @@ -92,17 +103,10 @@ async fn from_response(ctx: &mut RequestContext, exec_ctx: &ExecutionContext) -> /// Hydrates `ctx` from the conversation store. /// -/// Gets or creates the conversation (depending on `store`) and rehydrates its /// history in parallel, then prepends the history items to the enriched request input. async fn from_conversation(ctx: &mut RequestContext, exec_ctx: &ExecutionContext) -> ExecutorResult<()> { let (conv_data, snapshot) = tokio::try_join!( - async { - if ctx.original_request.store { - exec_ctx.conv_handler.get_or_create(ctx).await - } else { - exec_ctx.conv_handler.get(ctx).await - } - }, + async { exec_ctx.conv_handler.get(ctx).await }, exec_ctx.conv_handler.rehydrate_snapshot(ctx), )?; diff --git a/crates/agentic-server-core/src/executor/request.rs b/crates/agentic-server-core/src/executor/request.rs index e7e1cc9b..47fce39f 100644 --- a/crates/agentic-server-core/src/executor/request.rs +++ b/crates/agentic-server-core/src/executor/request.rs @@ -35,6 +35,7 @@ pub struct RequestContext { /// Conversation version captured with rehydrated history. /// `None` for non-conversation and `previous_response_id` execution. pub conversation_version: Option, + pub tenant_id: Option, } impl RequestContext { diff --git a/crates/agentic-server-core/src/executor/upstream.rs b/crates/agentic-server-core/src/executor/upstream.rs index 14eea041..3076a70c 100644 --- a/crates/agentic-server-core/src/executor/upstream.rs +++ b/crates/agentic-server-core/src/executor/upstream.rs @@ -410,6 +410,6 @@ fn apply_context_response_ids(wire: &mut WireEvent, ctx: &RequestContext) { ); } if let Some(conversation_id) = &ctx.conversation_id { - response.insert("conversation_id".to_owned(), Value::String(conversation_id.clone())); + response.insert("conversation".to_owned(), Value::String(conversation_id.clone())); } } diff --git a/crates/agentic-server-core/src/lib.rs b/crates/agentic-server-core/src/lib.rs index 8e7254da..7fb98c88 100644 --- a/crates/agentic-server-core/src/lib.rs +++ b/crates/agentic-server-core/src/lib.rs @@ -10,8 +10,9 @@ pub mod types; pub mod utils; pub use storage::{ - ConversationData, ConversationStore, DatabaseBackend, DbPool, InOutItem, ItemKind, ResponseData, ResponseMetadata, - ResponseStore, SchemaManager, StorageError, StoreResult, create_pool, create_pool_with_schema, + ConversationData, ConversationItemData, ConversationItemPage, ConversationStore, DatabaseBackend, DbPool, + InOutItem, ItemKind, ResponseData, ResponseMetadata, ResponseStore, SchemaManager, StorageError, StoreResult, + create_pool, create_pool_with_schema, models::{Conversation as DbConversation, Item as DbItem, Response as DbResponse}, }; pub use tool::{ diff --git a/crates/agentic-server-core/src/storage/conversation.rs b/crates/agentic-server-core/src/storage/conversation.rs index 4792dec2..fe8ce336 100644 --- a/crates/agentic-server-core/src/storage/conversation.rs +++ b/crates/agentic-server-core/src/storage/conversation.rs @@ -1,12 +1,17 @@ //! Conversation storage operations. +#![allow(clippy::missing_errors_doc)] + use std::convert::TryFrom; use std::sync::Arc; +use serde_json::Value; + use super::models::{conversation, item, response}; use super::pool::DbPool; use super::types::{ - ConversationData, ConversationSnapshot, ConversationVersion, InOutItem, ResponseMetadata, StorageError, StoreResult, + ConversationData, ConversationItemData, ConversationItemPage, ConversationSnapshot, ConversationVersion, InOutItem, + ResponseMetadata, StorageError, StoreResult, }; use crate::utils::common::{serialize_to_string, uuid7_str}; @@ -44,8 +49,27 @@ impl ConversationStore { /// /// Returns error if database query fails. pub async fn create(&self) -> StoreResult { + self.create_with_items_for_tenant(None, None, Vec::new()).await + } + + pub async fn create_with_items_for_tenant( + &self, + tenant_id: Option<&str>, + metadata: Option<&Value>, + initial_items: Vec, + ) -> StoreResult { let pool = self.pool()?; - let row = conversation::create(pool, &uuid7_str("conv_")).await?; + let conversation_id = uuid7_str("conv_"); + let metadata_json = metadata.map(serialize_to_string).transpose()?; + let items = serialize_items(initial_items)?; + let mut tx = pool.begin().await?; + let row = + conversation::create_with_metadata_in_tx(&mut tx, &conversation_id, tenant_id, metadata_json.as_deref()) + .await?; + if !items.is_empty() { + item::create_in_tx_with_tenant(&mut tx, items, Some(&conversation_id), tenant_id).await?; + } + tx.commit().await?; Ok(row.into()) } @@ -55,8 +79,16 @@ impl ConversationStore { /// /// Returns error if database query fails. pub async fn get_or_create(&self, conversation_id: &str) -> StoreResult { + self.get_or_create_for_tenant(conversation_id, None).await + } + + pub async fn get_or_create_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult { let pool = self.pool()?; - let row = conversation::get_or_create(pool, conversation_id).await?; + let row = conversation::get_or_create_for_tenant(pool, conversation_id, tenant_id).await?; Ok(row.into()) } @@ -66,13 +98,127 @@ impl ConversationStore { /// /// Returns error if conversation not found or database query fails. pub async fn get(&self, conversation_id: &str) -> StoreResult { + self.get_for_tenant(conversation_id, None).await + } + + pub async fn get_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult { + let pool = self.pool()?; + let row = conversation::get_for_tenant(pool, conversation_id, tenant_id) + .await? + .ok_or_else(|| StorageError::not_found("Conversation", conversation_id))?; + Ok(row.into()) + } + + pub async fn update_metadata_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + metadata: Option<&Value>, + ) -> StoreResult { let pool = self.pool()?; - let row = conversation::get(pool, conversation_id) + let metadata_json = metadata.map(serialize_to_string).transpose()?; + let row = conversation::update_metadata(pool, conversation_id, tenant_id, metadata_json.as_deref()) .await? .ok_or_else(|| StorageError::not_found("Conversation", conversation_id))?; Ok(row.into()) } + pub async fn delete_for_tenant(&self, conversation_id: &str, tenant_id: Option<&str>) -> StoreResult<()> { + let pool = self.pool()?; + if !conversation::delete(pool, conversation_id, tenant_id).await? { + return Err(StorageError::not_found("Conversation", conversation_id)); + } + Ok(()) + } + + pub async fn append_items_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + items: Vec, + ) -> StoreResult> { + let pool = self.pool()?; + self.get_for_tenant(conversation_id, tenant_id).await?; + let serialized = serialize_items(items)?; + let mut tx = pool.begin().await?; + match conversation::lock_in_tx_for_tenant(&mut tx, conversation_id, tenant_id).await { + Ok(()) => {} + Err(sqlx::Error::RowNotFound) => { + return Err(StorageError::not_found("Conversation", conversation_id)); + } + Err(error) => return Err(error.into()), + } + let rows = item::create_in_tx_with_tenant(&mut tx, serialized, Some(conversation_id), tenant_id).await?; + tx.commit().await?; + Ok(rows.into_iter().map(ConversationItemData::from).collect()) + } + + pub async fn list_items_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + after: Option<&str>, + limit: usize, + descending: bool, + ) -> StoreResult { + let pool = self.pool()?; + self.get_for_tenant(conversation_id, tenant_id).await?; + let mut rows = item::get_items_by_conversation_for_tenant(pool, conversation_id, tenant_id).await?; + if descending { + rows.reverse(); + } + let start = match after { + Some(after) => rows + .iter() + .position(|row| row.id == after) + .map(|index| index + 1) + .ok_or_else(|| StorageError::not_found("Conversation item", after))?, + None => 0, + }; + let end = start.saturating_add(limit).min(rows.len()); + let has_more = end < rows.len(); + Ok(ConversationItemPage { + data: rows[start..end] + .iter() + .cloned() + .map(ConversationItemData::from) + .collect(), + has_more, + }) + } + + pub async fn get_item_for_tenant( + &self, + conversation_id: &str, + item_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult { + let pool = self.pool()?; + self.get_for_tenant(conversation_id, tenant_id).await?; + let row = item::get_item(pool, conversation_id, item_id, tenant_id) + .await? + .ok_or_else(|| StorageError::not_found("Conversation item", item_id))?; + Ok(row.into()) + } + + pub async fn delete_item_for_tenant( + &self, + conversation_id: &str, + item_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult<()> { + let pool = self.pool()?; + self.get_for_tenant(conversation_id, tenant_id).await?; + if !item::delete(pool, conversation_id, item_id, tenant_id).await? { + return Err(StorageError::not_found("Conversation item", item_id)); + } + Ok(()) + } + /// Rehydrates a conversation with all its items. /// /// # Errors @@ -88,8 +234,17 @@ impl ConversationStore { /// /// Returns an error if a stored item is missing its sequence number or if the database query fails. pub async fn rehydrate_snapshot(&self, conversation_id: &str) -> StoreResult { + self.rehydrate_snapshot_for_tenant(conversation_id, None).await + } + + pub async fn rehydrate_snapshot_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult { let pool = self.pool()?; - let rows = item::get_items_by_conversation(pool, conversation_id).await?; + self.get_for_tenant(conversation_id, tenant_id).await?; + let rows = item::get_items_by_conversation_for_tenant(pool, conversation_id, tenant_id).await?; let mut last_sequence = None; for row in &rows { @@ -123,6 +278,7 @@ impl ConversationStore { self.persist_impl( conversation_id, None, + None, response_id, previous_response_id, new_items, @@ -147,6 +303,30 @@ impl ConversationStore { ) -> StoreResult<()> { self.persist_impl( conversation_id, + None, + Some(expected_version), + response_id, + previous_response_id, + new_items, + metadata, + ) + .await + } + + #[allow(clippy::too_many_arguments)] + pub async fn persist_if_version_for_tenant( + &self, + conversation_id: &str, + tenant_id: Option<&str>, + expected_version: ConversationVersion, + response_id: &str, + previous_response_id: Option<&str>, + new_items: Vec, + metadata: &ResponseMetadata, + ) -> StoreResult<()> { + self.persist_impl( + conversation_id, + tenant_id, Some(expected_version), response_id, previous_response_id, @@ -156,9 +336,11 @@ impl ConversationStore { .await } + #[allow(clippy::too_many_arguments)] async fn persist_impl( &self, conversation_id: &str, + tenant_id: Option<&str>, expected_version: Option, response_id: &str, previous_response_id: Option<&str>, @@ -167,20 +349,14 @@ impl ConversationStore { ) -> StoreResult<()> { let pool = self.pool()?; - let mut item_ids: Vec = Vec::new(); - let mut items_: Vec<(String, String)> = Vec::new(); - for any_item in new_items { - let item_id = uuid7_str("item_"); - item_ids.push(item_id.clone()); - let data_str = String::try_from(&any_item)?; - items_.push((item_id, data_str)); - } + let items_ = serialize_items(new_items)?; + let item_ids: Vec = items_.iter().map(|(id, _)| id.clone()).collect(); let history_item_ids_json = serialize_to_string(&item_ids)?; let metadata_json = String::try_from(metadata)?; let mut tx = pool.begin().await?; - match conversation::lock_in_tx(&mut tx, conversation_id).await { + match conversation::lock_in_tx_for_tenant(&mut tx, conversation_id, tenant_id).await { Ok(()) => {} Err(sqlx::Error::RowNotFound) => { return Err(StorageError::not_found("Conversation", conversation_id)); @@ -189,7 +365,7 @@ impl ConversationStore { } if let Some(expected_version) = expected_version { let current_version = ConversationVersion::from_last_sequence( - item::last_conversation_sequence_in_tx(&mut tx, conversation_id).await?, + item::last_conversation_sequence_in_tx_for_tenant(&mut tx, conversation_id, tenant_id).await?, ); if current_version != expected_version { return Err(StorageError::ConversationConflict { @@ -197,15 +373,16 @@ impl ConversationStore { }); } } - item::create_in_tx(&mut tx, items_, Some(conversation_id)).await?; + item::create_in_tx_with_tenant(&mut tx, items_, Some(conversation_id), tenant_id).await?; - response::create_in_tx( + response::create_in_tx_with_tenant( &mut tx, response_id, Some(conversation_id), previous_response_id, Some(&history_item_ids_json), Some(&metadata_json), + tenant_id, ) .await?; tx.commit().await?; @@ -213,3 +390,14 @@ impl ConversationStore { Ok(()) } } + +fn serialize_items(items: Vec) -> StoreResult> { + items + .into_iter() + .map(|item| { + let id = uuid7_str("item_"); + let data = String::try_from(&item)?; + Ok((id, data)) + }) + .collect() +} diff --git a/crates/agentic-server-core/src/storage/mod.rs b/crates/agentic-server-core/src/storage/mod.rs index 3a44cf02..daa88404 100644 --- a/crates/agentic-server-core/src/storage/mod.rs +++ b/crates/agentic-server-core/src/storage/mod.rs @@ -34,6 +34,6 @@ pub use pool::{ pub use response::ResponseStore; pub use schema::{PoolWithSchema, SchemaManager}; pub use types::{ - ConversationData, ConversationSnapshot, ConversationVersion, InOutItem, ItemKind, ResponseData, ResponseMetadata, - StorageError, StoreResult, + ConversationData, ConversationItemData, ConversationItemPage, ConversationSnapshot, ConversationVersion, InOutItem, + ItemKind, ResponseData, ResponseMetadata, StorageError, StoreResult, }; diff --git a/crates/agentic-server-core/src/storage/models/conversation.rs b/crates/agentic-server-core/src/storage/models/conversation.rs index b9f4de01..2b93a06a 100644 --- a/crates/agentic-server-core/src/storage/models/conversation.rs +++ b/crates/agentic-server-core/src/storage/models/conversation.rs @@ -1,5 +1,7 @@ //! Conversation context and history. +#![allow(clippy::missing_errors_doc)] + use super::super::pool::{DbPool, DbResult, DbTransaction}; use crate::storage::backend::DatabaseBackend; use crate::utils::common::utcnow_str; @@ -13,6 +15,8 @@ pub struct Conversation { /// Unique conversation identifier. pub id: String, + pub tenant_id: Option, + /// Optional metadata as JSON string. pub metadata: Option, @@ -25,14 +29,37 @@ pub struct Conversation { /// # Errors /// Returns `DbResult::Err` if the database insertion fails. pub async fn create(pool: &DbPool, id: &str) -> DbResult { + create_with_metadata(pool, id, None, None).await +} + +pub async fn create_with_metadata( + pool: &DbPool, + id: &str, + tenant_id: Option<&str>, + metadata: Option<&str>, +) -> DbResult { + let mut tx = pool.begin().await?; + let conversation = create_with_metadata_in_tx(&mut tx, id, tenant_id, metadata).await?; + tx.commit().await?; + Ok(conversation) +} + +pub async fn create_with_metadata_in_tx( + tx: &mut DbTransaction<'_>, + id: &str, + tenant_id: Option<&str>, + metadata: Option<&str>, +) -> DbResult { let now = utcnow_str(); sqlx::query_as::<_, Conversation>( - "INSERT INTO conversations (id, created_at) \ - VALUES ($1, $2) RETURNING *", + "INSERT INTO conversations (id, created_at, tenant_id, metadata) \ + VALUES ($1, $2, $3, $4) RETURNING *", ) .bind(id) .bind(now) - .fetch_one(pool) + .bind(tenant_id) + .bind(metadata) + .fetch_one(&mut **tx) .await } @@ -41,17 +68,37 @@ pub async fn create(pool: &DbPool, id: &str) -> DbResult { /// # Errors /// Returns `DbResult::Err` if the database query fails. pub async fn get_or_create(pool: &DbPool, id: &str) -> DbResult { + get_or_create_for_tenant(pool, id, None).await +} + +pub async fn get_or_create_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult { let now = utcnow_str(); - sqlx::query_as::<_, Conversation>( - "INSERT INTO conversations (id, created_at) \ - VALUES ($1, $2) \ - ON CONFLICT (id) DO UPDATE SET created_at = created_at \ - RETURNING *", - ) - .bind(id) - .bind(now) - .fetch_one(pool) - .await + if let Some(tenant_id) = tenant_id { + sqlx::query_as::<_, Conversation>( + "INSERT INTO conversations (id, created_at, tenant_id) \ + VALUES ($1, $2, $3) \ + ON CONFLICT (id) DO UPDATE SET created_at = conversations.created_at \ + WHERE conversations.tenant_id = $3 \ + RETURNING *", + ) + .bind(id) + .bind(now) + .bind(tenant_id) + .fetch_one(pool) + .await + } else { + sqlx::query_as::<_, Conversation>( + "INSERT INTO conversations (id, created_at, tenant_id) \ + VALUES ($1, $2, NULL) \ + ON CONFLICT (id) DO UPDATE SET created_at = conversations.created_at \ + WHERE conversations.tenant_id IS NULL \ + RETURNING *", + ) + .bind(id) + .bind(now) + .fetch_one(pool) + .await + } } /// Get a conversation by ID. @@ -59,10 +106,101 @@ pub async fn get_or_create(pool: &DbPool, id: &str) -> DbResult { /// # Errors /// Returns `DbResult::Err` if the database query fails. pub async fn get(pool: &DbPool, id: &str) -> DbResult> { - sqlx::query_as::<_, Conversation>("SELECT * FROM conversations WHERE id = $1") - .bind(id) - .fetch_optional(pool) - .await + get_for_tenant(pool, id, None).await +} + +pub async fn get_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_as::<_, Conversation>("SELECT * FROM conversations WHERE id = $1 AND tenant_id = $2") + .bind(id) + .bind(tenant_id) + .fetch_optional(pool) + .await + } + None => { + sqlx::query_as::<_, Conversation>("SELECT * FROM conversations WHERE id = $1 AND tenant_id IS NULL") + .bind(id) + .fetch_optional(pool) + .await + } + } +} + +pub async fn update_metadata( + pool: &DbPool, + id: &str, + tenant_id: Option<&str>, + metadata: Option<&str>, +) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_as::<_, Conversation>( + "UPDATE conversations SET metadata = $3 \ + WHERE id = $1 AND tenant_id = $2 RETURNING *", + ) + .bind(id) + .bind(tenant_id) + .bind(metadata) + .fetch_optional(pool) + .await + } + None => { + sqlx::query_as::<_, Conversation>( + "UPDATE conversations SET metadata = $2 \ + WHERE id = $1 AND tenant_id IS NULL RETURNING *", + ) + .bind(id) + .bind(metadata) + .fetch_optional(pool) + .await + } + } +} + +pub async fn delete(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult { + let mut tx = pool.begin().await?; + match tenant_id { + Some(tenant_id) => { + sqlx::query( + "UPDATE items SET conversation_id = NULL, seq = NULL \ + WHERE conversation_id = $1 AND tenant_id = $2", + ) + .bind(id) + .bind(tenant_id) + .execute(&mut *tx) + .await?; + } + None => { + sqlx::query( + "UPDATE items SET conversation_id = NULL, seq = NULL \ + WHERE conversation_id = $1 AND tenant_id IS NULL", + ) + .bind(id) + .execute(&mut *tx) + .await?; + } + } + let result = match tenant_id { + Some(tenant_id) => { + sqlx::query("DELETE FROM conversations WHERE id = $1 AND tenant_id = $2") + .bind(id) + .bind(tenant_id) + .execute(&mut *tx) + .await? + } + None => { + sqlx::query("DELETE FROM conversations WHERE id = $1 AND tenant_id IS NULL") + .bind(id) + .execute(&mut *tx) + .await? + } + }; + let deleted = result.rows_affected() > 0; + if deleted { + tx.commit().await?; + } + Ok(deleted) } /// Locks an existing conversation for the lifetime of the transaction. @@ -75,18 +213,48 @@ pub async fn get(pool: &DbPool, id: &str) -> DbResult> { /// # Errors /// Returns `DbResult::Err` if the database query fails or the conversation does not exist. pub async fn lock_in_tx(tx: &mut DbTransaction<'_>, id: &str) -> DbResult<()> { + lock_in_tx_for_tenant(tx, id, None).await +} + +pub async fn lock_in_tx_for_tenant(tx: &mut DbTransaction<'_>, id: &str, tenant_id: Option<&str>) -> DbResult<()> { if DatabaseBackend::from_connection(tx.as_mut()) == DatabaseBackend::Postgres { - let locked_id = sqlx::query_scalar::<_, String>("SELECT id FROM conversations WHERE id = $1 FOR UPDATE") - .bind(id) - .fetch_optional(&mut **tx) - .await?; + let locked_id = match tenant_id { + Some(tenant_id) => { + sqlx::query_scalar::<_, String>( + "SELECT id FROM conversations WHERE id = $1 AND tenant_id = $2 FOR UPDATE", + ) + .bind(id) + .bind(tenant_id) + .fetch_optional(&mut **tx) + .await? + } + None => { + sqlx::query_scalar::<_, String>( + "SELECT id FROM conversations WHERE id = $1 AND tenant_id IS NULL FOR UPDATE", + ) + .bind(id) + .fetch_optional(&mut **tx) + .await? + } + }; return locked_id.map(|_| ()).ok_or(sqlx::Error::RowNotFound); } - let result = sqlx::query("UPDATE conversations SET created_at = created_at WHERE id = $1") - .bind(id) - .execute(&mut **tx) - .await?; + let result = match tenant_id { + Some(tenant_id) => { + sqlx::query("UPDATE conversations SET created_at = created_at WHERE id = $1 AND tenant_id = $2") + .bind(id) + .bind(tenant_id) + .execute(&mut **tx) + .await? + } + None => { + sqlx::query("UPDATE conversations SET created_at = created_at WHERE id = $1 AND tenant_id IS NULL") + .bind(id) + .execute(&mut **tx) + .await? + } + }; if result.rows_affected() == 0 { return Err(sqlx::Error::RowNotFound); } @@ -101,6 +269,7 @@ mod tests { fn test_conversation_basic() { let conversation = Conversation { id: "conv_1".to_string(), + tenant_id: None, metadata: None, created_at: 1_704_067_200, }; diff --git a/crates/agentic-server-core/src/storage/models/item.rs b/crates/agentic-server-core/src/storage/models/item.rs index e6cc81a8..6b31a5a7 100644 --- a/crates/agentic-server-core/src/storage/models/item.rs +++ b/crates/agentic-server-core/src/storage/models/item.rs @@ -1,5 +1,7 @@ //! Conversation history item stored in the database. +#![allow(clippy::missing_errors_doc)] + use serde_json::Value; use std::fmt::Write; use tracing::warn; @@ -9,7 +11,7 @@ use super::super::types::item::{InOutItem, ItemKind, STORED_ITEM_KIND_KEY}; use crate::types::io::{InputItem, OutputItem}; use crate::utils::common::{deserialize_from_str_opt, utcnow_str}; -const ITEM_COLUMN_COUNT: usize = 5; +const ITEM_COLUMN_COUNT: usize = 6; const SEQUENCE_COLUMN_INDEX: usize = 4; const MAX_BIND_PARAMETERS: usize = 999; const MAX_ITEMS_PER_INSERT: usize = MAX_BIND_PARAMETERS / ITEM_COLUMN_COUNT; @@ -35,6 +37,8 @@ pub struct Item { /// Optional sequence number within conversation. pub seq: Option, + + pub tenant_id: Option, } impl Item { @@ -131,6 +135,15 @@ pub async fn create_in_tx( tx: &mut DbTransaction<'_>, items: Vec<(String, String)>, conversation_id: Option<&str>, +) -> DbResult> { + create_in_tx_with_tenant(tx, items, conversation_id, None).await +} + +pub async fn create_in_tx_with_tenant( + tx: &mut DbTransaction<'_>, + items: Vec<(String, String)>, + conversation_id: Option<&str>, + tenant_id: Option<&str>, ) -> DbResult> { if items.is_empty() { return Ok(Vec::new()); @@ -139,9 +152,9 @@ pub async fn create_in_tx( let mut created = Vec::with_capacity(items.len()); for batch in items.chunks(MAX_ITEMS_PER_INSERT) { let mut rows = if let Some(conversation_id) = conversation_id { - create_in_tx_with_next_conversation_seq(tx, batch, conversation_id).await? + create_in_tx_with_next_conversation_seq(tx, batch, conversation_id, tenant_id).await? } else { - create_in_tx_without_conversation(tx, batch).await? + create_in_tx_without_conversation(tx, batch, tenant_id).await? }; created.append(&mut rows); } @@ -151,15 +164,23 @@ pub async fn create_in_tx( async fn create_in_tx_without_conversation( tx: &mut DbTransaction<'_>, items: &[(String, String)], + tenant_id: Option<&str>, ) -> DbResult> { let now = utcnow_str(); let values_clause = item_values_clause(items.len(), 1, false); - let sql = - format!("INSERT INTO items (id, data, created_at, conversation_id, seq) VALUES {values_clause} RETURNING *"); + let sql = format!( + "INSERT INTO items (id, data, created_at, conversation_id, seq, tenant_id) VALUES {values_clause} RETURNING *" + ); let mut query = sqlx::query_as::<_, Item>(&sql); for (id, data) in items { - query = query.bind(id).bind(data).bind(now).bind(None::<&str>).bind(None::); + query = query + .bind(id) + .bind(data) + .bind(now) + .bind(None::<&str>) + .bind(None::) + .bind(tenant_id); } query.fetch_all(&mut **tx).await @@ -169,21 +190,31 @@ async fn create_in_tx_with_next_conversation_seq( tx: &mut DbTransaction<'_>, items: &[(String, String)], conversation_id: &str, + tenant_id: Option<&str>, ) -> DbResult> { let now = utcnow_str(); - let values_clause = item_values_clause(items.len(), 2, true); + let (values_first_bind, sequence_query) = match tenant_id { + Some(_) => ( + 3, + "SELECT COALESCE(MAX(seq), -1) + 1 AS start FROM items WHERE conversation_id = $1 AND tenant_id = $2", + ), + None => ( + 2, + "SELECT COALESCE(MAX(seq), -1) + 1 AS start FROM items WHERE conversation_id = $1 AND tenant_id IS NULL", + ), + }; + let values_clause = item_values_clause(items.len(), values_first_bind, true); let sql = format!( - "WITH next_seq AS ( \ - SELECT COALESCE(MAX(seq), -1) + 1 AS start \ - FROM items \ - WHERE conversation_id = $1 \ - ) \ - INSERT INTO items (id, data, created_at, conversation_id, seq) \ + "WITH next_seq AS ({sequence_query}) \ + INSERT INTO items (id, data, created_at, conversation_id, seq, tenant_id) \ VALUES {values_clause} \ RETURNING *" ); let mut query = sqlx::query_as::<_, Item>(&sql).bind(conversation_id); + if let Some(tenant_id) = tenant_id { + query = query.bind(tenant_id); + } #[allow(clippy::cast_possible_wrap)] for (idx, (id, data)) in items.iter().enumerate() { query = query @@ -191,7 +222,8 @@ async fn create_in_tx_with_next_conversation_seq( .bind(data) .bind(now) .bind(conversation_id) - .bind(idx as i64); + .bind(idx as i64) + .bind(tenant_id); } query.fetch_all(&mut **tx).await @@ -202,34 +234,119 @@ async fn create_in_tx_with_next_conversation_seq( /// # Errors /// Returns `DbResult::Err` if the database query fails. pub async fn get_items(pool: &DbPool, ids: &[String]) -> DbResult> { + get_items_for_tenant(pool, ids, None).await +} + +pub async fn get_items_for_tenant(pool: &DbPool, ids: &[String], tenant_id: Option<&str>) -> DbResult> { if ids.is_empty() { return Ok(vec![]); } let mut rows = Vec::with_capacity(ids.len()); - for batch in ids.chunks(MAX_BIND_PARAMETERS) { + let max_ids_per_batch = if tenant_id.is_some() { + MAX_BIND_PARAMETERS - 1 + } else { + MAX_BIND_PARAMETERS + }; + for batch in ids.chunks(max_ids_per_batch) { let placeholders = (1..=batch.len()) .map(|index| format!("${index}")) .collect::>() .join(", "); - let sql = format!("SELECT * FROM items WHERE id IN ({placeholders})"); + let sql = match tenant_id { + Some(_) => format!( + "SELECT * FROM items WHERE id IN ({placeholders}) AND tenant_id = ${}", + batch.len() + 1 + ), + None => format!("SELECT * FROM items WHERE id IN ({placeholders}) AND tenant_id IS NULL"), + }; let mut query = sqlx::query_as::<_, Item>(&sql); for id in batch { query = query.bind(id); } + if let Some(tenant_id) = tenant_id { + query = query.bind(tenant_id); + } rows.extend(query.fetch_all(pool).await?); } Ok(rows) } -/// Get items by conversation ID ordered by sequence. -/// -/// # Errors -/// Returns `DbResult::Err` if the database query fails. +pub async fn get_item( + pool: &DbPool, + conversation_id: &str, + item_id: &str, + tenant_id: Option<&str>, +) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_as::<_, Item>("SELECT * FROM items WHERE id = $1 AND conversation_id = $2 AND tenant_id = $3") + .bind(item_id) + .bind(conversation_id) + .bind(tenant_id) + .fetch_optional(pool) + .await + } + None => { + sqlx::query_as::<_, Item>( + "SELECT * FROM items WHERE id = $1 AND conversation_id = $2 AND tenant_id IS NULL", + ) + .bind(item_id) + .bind(conversation_id) + .fetch_optional(pool) + .await + } + } +} + pub async fn get_items_by_conversation(pool: &DbPool, conversation_id: &str) -> DbResult> { - sqlx::query_as::<_, Item>("SELECT * FROM items WHERE conversation_id = $1 ORDER BY seq ASC") - .bind(conversation_id) - .fetch_all(pool) - .await + get_items_by_conversation_for_tenant(pool, conversation_id, None).await +} + +pub async fn get_items_by_conversation_for_tenant( + pool: &DbPool, + conversation_id: &str, + tenant_id: Option<&str>, +) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_as::<_, Item>( + "SELECT * FROM items WHERE conversation_id = $1 AND tenant_id = $2 ORDER BY seq ASC", + ) + .bind(conversation_id) + .bind(tenant_id) + .fetch_all(pool) + .await + } + None => { + sqlx::query_as::<_, Item>( + "SELECT * FROM items WHERE conversation_id = $1 AND tenant_id IS NULL ORDER BY seq ASC", + ) + .bind(conversation_id) + .fetch_all(pool) + .await + } + } +} + +pub async fn delete(pool: &DbPool, conversation_id: &str, item_id: &str, tenant_id: Option<&str>) -> DbResult { + let result = match tenant_id { + Some(tenant_id) => { + sqlx::query("DELETE FROM items WHERE id = $1 AND conversation_id = $2 AND tenant_id = $3") + .bind(item_id) + .bind(conversation_id) + .bind(tenant_id) + .execute(pool) + .await? + } + None => { + sqlx::query("DELETE FROM items WHERE id = $1 AND conversation_id = $2 AND tenant_id IS NULL") + .bind(item_id) + .bind(conversation_id) + .execute(pool) + .await? + } + }; + Ok(result.rows_affected() > 0) } /// Returns the last stored item sequence for a conversation inside a transaction. @@ -240,10 +357,29 @@ pub async fn last_conversation_sequence_in_tx( tx: &mut DbTransaction<'_>, conversation_id: &str, ) -> DbResult> { - sqlx::query_scalar("SELECT MAX(seq) FROM items WHERE conversation_id = $1") - .bind(conversation_id) - .fetch_one(&mut **tx) - .await + last_conversation_sequence_in_tx_for_tenant(tx, conversation_id, None).await +} + +pub async fn last_conversation_sequence_in_tx_for_tenant( + tx: &mut DbTransaction<'_>, + conversation_id: &str, + tenant_id: Option<&str>, +) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_scalar("SELECT MAX(seq) FROM items WHERE conversation_id = $1 AND tenant_id = $2") + .bind(conversation_id) + .bind(tenant_id) + .fetch_one(&mut **tx) + .await + } + None => { + sqlx::query_scalar("SELECT MAX(seq) FROM items WHERE conversation_id = $1 AND tenant_id IS NULL") + .bind(conversation_id) + .fetch_one(&mut **tx) + .await + } + } } #[cfg(test)] @@ -256,7 +392,7 @@ mod tests { fn item_values_clause_numbers_plain_rows() { assert_eq!( item_values_clause(2, 1, false), - "($1, $2, $3, $4, $5), ($6, $7, $8, $9, $10)" + "($1, $2, $3, $4, $5, $6), ($7, $8, $9, $10, $11, $12)" ); } @@ -264,8 +400,8 @@ mod tests { fn item_values_clause_numbers_conversation_rows_after_cte_bind() { assert_eq!( item_values_clause(2, 2, true), - "($2, $3, $4, $5, (SELECT start + $6 FROM next_seq)), \ - ($7, $8, $9, $10, (SELECT start + $11 FROM next_seq))" + "($2, $3, $4, $5, (SELECT start + $6 FROM next_seq), $7), \ + ($8, $9, $10, $11, (SELECT start + $12 FROM next_seq), $13)" ); } @@ -336,6 +472,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: Some("conv_456".to_string()), seq: Some(1), + tenant_id: None, }; assert_eq!(item.id, "item_123"); @@ -351,6 +488,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: None, + tenant_id: None, }; assert!(item.conversation_id.is_none()); @@ -368,6 +506,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: None, + tenant_id: None, }; assert!(matches!( @@ -391,6 +530,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: None, + tenant_id: None, }; let stored = item.as_inout().expect("stored item"); @@ -416,6 +556,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: None, + tenant_id: None, }; let inputs = InOutItem::into_input_items(vec![item.as_inout().expect("stored item")]); @@ -459,6 +600,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: Some(idx.try_into().expect("seq")), + tenant_id: None, }) .map(|item| item.as_inout().expect("stored item")) .collect(); @@ -492,6 +634,7 @@ mod tests { created_at: 1_704_067_200, conversation_id: None, seq: None, + tenant_id: None, }; let inputs = InOutItem::into_input_items(vec![item.as_inout().expect("stored item")]); diff --git a/crates/agentic-server-core/src/storage/models/response.rs b/crates/agentic-server-core/src/storage/models/response.rs index a613b82d..fb37c57b 100644 --- a/crates/agentic-server-core/src/storage/models/response.rs +++ b/crates/agentic-server-core/src/storage/models/response.rs @@ -1,5 +1,7 @@ //! LLM API response stored in the database. +#![allow(clippy::missing_errors_doc)] + use super::super::pool::{DbPool, DbResult, DbTransaction}; use crate::utils::common::{deserialize_from_string_opt, deserialize_from_string_opt_or_default, utcnow_str}; @@ -12,6 +14,8 @@ pub struct Response { /// Unique response identifier. pub id: String, + pub tenant_id: Option, + /// Optional conversation this response belongs to. pub conversation_id: Option, @@ -39,12 +43,33 @@ pub async fn create_in_tx( previous_response_id: Option<&str>, history_item_ids: Option<&str>, metadata: Option<&str>, +) -> DbResult { + create_in_tx_with_tenant( + tx, + id, + conversation_id, + previous_response_id, + history_item_ids, + metadata, + None, + ) + .await +} + +pub async fn create_in_tx_with_tenant( + tx: &mut DbTransaction<'_>, + id: &str, + conversation_id: Option<&str>, + previous_response_id: Option<&str>, + history_item_ids: Option<&str>, + metadata: Option<&str>, + tenant_id: Option<&str>, ) -> DbResult { let now = utcnow_str(); sqlx::query_as::<_, Response>( "INSERT INTO responses \ - (id, conversation_id, previous_response_id, history_item_ids, metadata, created_at) \ - VALUES ($1, $2, $3, $4, $5, $6) RETURNING *", + (id, conversation_id, previous_response_id, history_item_ids, metadata, created_at, tenant_id) \ + VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING *", ) .bind(id) .bind(conversation_id) @@ -52,6 +77,7 @@ pub async fn create_in_tx( .bind(history_item_ids) .bind(metadata) .bind(now) + .bind(tenant_id) .fetch_one(&mut **tx) .await } @@ -61,10 +87,25 @@ pub async fn create_in_tx( /// # Errors /// Returns `DbResult::Err` if the database query fails. pub async fn get(pool: &DbPool, id: &str) -> DbResult> { - sqlx::query_as::<_, Response>("SELECT * FROM responses WHERE id = $1") - .bind(id) - .fetch_optional(pool) - .await + get_for_tenant(pool, id, None).await +} + +pub async fn get_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult> { + match tenant_id { + Some(tenant_id) => { + sqlx::query_as::<_, Response>("SELECT * FROM responses WHERE id = $1 AND tenant_id = $2") + .bind(id) + .bind(tenant_id) + .fetch_optional(pool) + .await + } + None => { + sqlx::query_as::<_, Response>("SELECT * FROM responses WHERE id = $1 AND tenant_id IS NULL") + .bind(id) + .fetch_optional(pool) + .await + } + } } impl Response { @@ -89,6 +130,7 @@ mod tests { fn test_response_history_ids_empty() { let response = Response { id: "test".to_string(), + tenant_id: None, conversation_id: None, previous_response_id: None, history_item_ids: None, @@ -104,6 +146,7 @@ mod tests { fn test_response_history_ids_valid() { let response = Response { id: "test".to_string(), + tenant_id: None, conversation_id: None, previous_response_id: None, history_item_ids: Some(r#"["item_1", "item_2"]"#.to_string()), @@ -125,6 +168,7 @@ mod tests { let response = Response { id: "resp_1".to_string(), + tenant_id: None, conversation_id: None, previous_response_id: None, history_item_ids: None, diff --git a/crates/agentic-server-core/src/storage/response.rs b/crates/agentic-server-core/src/storage/response.rs index def95991..1701af5c 100644 --- a/crates/agentic-server-core/src/storage/response.rs +++ b/crates/agentic-server-core/src/storage/response.rs @@ -1,5 +1,7 @@ //! Response storage operations and queries. +#![allow(clippy::missing_errors_doc)] + use std::collections::HashMap; use std::convert::TryFrom; use std::sync::Arc; @@ -49,8 +51,12 @@ impl ResponseStore { /// /// Returns error if response not found, database query fails, or store is disabled. pub async fn get(&self, response_id: &str) -> StoreResult { + self.get_for_tenant(response_id, None).await + } + + pub async fn get_for_tenant(&self, response_id: &str, tenant_id: Option<&str>) -> StoreResult { let pool = self.pool()?; - let row = response::get(pool, response_id) + let row = response::get_for_tenant(pool, response_id, tenant_id) .await? .ok_or_else(|| StorageError::not_found("Response", response_id))?; Ok(row.into()) @@ -64,9 +70,17 @@ impl ResponseStore { /// /// Returns error if database query fails or store is disabled. pub async fn rehydrate(&self, response_id: &str) -> StoreResult> { + self.rehydrate_for_tenant(response_id, None).await + } + + pub async fn rehydrate_for_tenant( + &self, + response_id: &str, + tenant_id: Option<&str>, + ) -> StoreResult> { let pool = self.pool()?; - let response = self.get(response_id).await?; - let rows = item::get_items(pool, &response.history_item_ids).await?; + let response = self.get_for_tenant(response_id, tenant_id).await?; + let rows = item::get_items_for_tenant(pool, &response.history_item_ids, tenant_id).await?; let mut items_by_id: HashMap = rows .into_iter() .filter_map(|row| { @@ -98,27 +112,23 @@ impl ResponseStore { new_items: Vec, metadata: &ResponseMetadata, ) -> StoreResult<()> { - self.persist_with_conversation_id(response_id, None, previous_response_id, new_items, metadata) + self.persist_with_conversation_id_for_tenant(response_id, None, previous_response_id, new_items, metadata, None) .await } - /// Persists a response while retaining its inherited conversation ID. - /// - /// # Errors - /// - /// Returns [`StorageError`] if database operation fails or store is disabled. - pub(crate) async fn persist_with_conversation_id( + pub(crate) async fn persist_with_conversation_id_for_tenant( &self, response_id: &str, conversation_id: Option<&str>, previous_response_id: Option<&str>, new_items: Vec, metadata: &ResponseMetadata, + tenant_id: Option<&str>, ) -> StoreResult<()> { let pool = self.pool()?; let mut item_ids: Vec = match previous_response_id { - Some(prev_id) => self.get(prev_id).await?.history_item_ids, + Some(prev_id) => self.get_for_tenant(prev_id, tenant_id).await?.history_item_ids, None => Vec::new(), }; let mut items_: Vec<(String, String)> = Vec::new(); @@ -133,15 +143,16 @@ impl ResponseStore { let mut tx = pool.begin().await?; - item::create_in_tx(&mut tx, items_, None).await?; + item::create_in_tx_with_tenant(&mut tx, items_, None, tenant_id).await?; - response::create_in_tx( + response::create_in_tx_with_tenant( &mut tx, response_id, conversation_id, previous_response_id, Some(&history_item_ids_json), Some(&metadata_json), + tenant_id, ) .await?; tx.commit().await?; diff --git a/crates/agentic-server-core/src/storage/types/conversation.rs b/crates/agentic-server-core/src/storage/types/conversation.rs index 71c0f590..4652992f 100644 --- a/crates/agentic-server-core/src/storage/types/conversation.rs +++ b/crates/agentic-server-core/src/storage/types/conversation.rs @@ -59,6 +59,7 @@ impl From for StorageDbConversation { id: data.conversation_id, metadata: data.metadata, created_at: data.created_at, + tenant_id: None, } } } @@ -73,6 +74,7 @@ mod tests { id: "conv_123".to_string(), metadata: None, created_at: 1_704_067_200, + tenant_id: None, }; let conversation: ConversationData = db_row.into(); diff --git a/crates/agentic-server-core/src/storage/types/item.rs b/crates/agentic-server-core/src/storage/types/item.rs index 5159d4ea..5e052a5e 100644 --- a/crates/agentic-server-core/src/storage/types/item.rs +++ b/crates/agentic-server-core/src/storage/types/item.rs @@ -6,8 +6,9 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use crate::storage::StorageError; +use crate::storage::models::Item as StorageDbItem; use crate::types::io::{InputItem, OutputItem}; -use crate::utils::common::serialize_to_value; +use crate::utils::common::{deserialize_from_str_opt, serialize_to_value}; pub(crate) const STORED_ITEM_KIND_KEY: &str = "_agentic_item_kind"; @@ -43,6 +44,20 @@ pub enum InOutItem { Output(OutputItem), } +#[derive(Debug, Clone, PartialEq)] +pub struct ConversationItemData { + pub id: String, + pub item: Value, + pub created_at: i64, + pub sequence: i64, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct ConversationItemPage { + pub data: Vec, + pub has_more: bool, +} + fn serialized_values_equal(left: &T, right: &T) -> bool { let Ok(left) = serialize_to_value(left) else { return false; @@ -116,6 +131,22 @@ impl InOutItem { } } +impl From for ConversationItemData { + fn from(row: StorageDbItem) -> Self { + let mut item = deserialize_from_str_opt::(&row.data).unwrap_or(Value::Null); + if let Some(object) = item.as_object_mut() { + object.remove(STORED_ITEM_KIND_KEY); + object.insert("id".to_owned(), Value::String(row.id.clone())); + } + Self { + id: row.id, + item, + created_at: row.created_at, + sequence: row.seq.unwrap_or_default(), + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/agentic-server-core/src/storage/types/mod.rs b/crates/agentic-server-core/src/storage/types/mod.rs index 41dfa538..193047ac 100644 --- a/crates/agentic-server-core/src/storage/types/mod.rs +++ b/crates/agentic-server-core/src/storage/types/mod.rs @@ -7,5 +7,5 @@ pub mod response; pub use conversation::{ConversationData, ConversationSnapshot, ConversationVersion}; pub use errors::{StorageError, StoreResult}; -pub use item::{InOutItem, ItemKind}; +pub use item::{ConversationItemData, ConversationItemPage, InOutItem, ItemKind}; pub use response::{ResponseData, ResponseMetadata}; diff --git a/crates/agentic-server-core/src/storage/types/response.rs b/crates/agentic-server-core/src/storage/types/response.rs index 9572f398..1d430465 100644 --- a/crates/agentic-server-core/src/storage/types/response.rs +++ b/crates/agentic-server-core/src/storage/types/response.rs @@ -83,6 +83,7 @@ mod tests { .to_string(), ), created_at: 1_704_067_200, + tenant_id: None, }; let response: ResponseData = db_row.into(); @@ -102,6 +103,7 @@ mod tests { history_item_ids: None, metadata: None, created_at: 1_704_067_200, + tenant_id: None, }; let response: ResponseData = db_row.into(); @@ -195,6 +197,7 @@ mod tests { history_item_ids: Some(r#"["item_1","item_2","item_3"]"#.to_string()), metadata: Some(r#"{"model":"gpt-3.5"}"#.to_string()), created_at: 1_704_067_200, + tenant_id: None, }; let response: ResponseData = db_row.into(); diff --git a/crates/agentic-server-core/src/types/request_response.rs b/crates/agentic-server-core/src/types/request_response.rs index 9076a320..28512cd9 100644 --- a/crates/agentic-server-core/src/types/request_response.rs +++ b/crates/agentic-server-core/src/types/request_response.rs @@ -11,12 +11,13 @@ use super::tools::{CustomToolParam, ResponsesTool}; use crate::tool::{CodexNamespaceHandler, ToolError}; use crate::utils::common::serialize_to_string; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct RequestPayload { pub model: String, pub input: ResponsesInput, pub instructions: Option, pub previous_response_id: Option, + #[serde(rename = "conversation")] pub conversation_id: Option, pub tools: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -38,6 +39,75 @@ pub struct RequestPayload { pub context_management: Option>, } +#[derive(Debug, Deserialize)] +struct RequestPayloadWire { + model: String, + input: ResponsesInput, + instructions: Option, + previous_response_id: Option, + conversation: Option, + #[serde(rename = "conversation_id")] + legacy_conversation_id: Option, + tools: Option>, + #[serde(default)] + tool_choice: Option, + #[serde(default)] + stream: bool, + #[serde(default = "default_true")] + store: bool, + include: Option>, + temperature: Option, + top_p: Option, + max_output_tokens: Option, + truncation: Option, + metadata: Option, + parallel_tool_calls: Option, + #[serde(default)] + cache_salt: Option, + #[serde(default)] + context_management: Option>, +} + +impl<'de> Deserialize<'de> for RequestPayload { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let wire = RequestPayloadWire::deserialize(deserializer)?; + let conversation_id = match (wire.conversation, wire.legacy_conversation_id) { + (Some(conversation), Some(legacy_conversation)) if conversation != legacy_conversation => { + return Err(serde::de::Error::custom( + "conversation and conversation_id must reference the same conversation", + )); + } + (Some(conversation), _) => Some(conversation), + (_, Some(legacy_conversation)) => Some(legacy_conversation), + (None, None) => None, + }; + + Ok(Self { + model: wire.model, + input: wire.input, + instructions: wire.instructions, + previous_response_id: wire.previous_response_id, + conversation_id, + tools: wire.tools, + tool_choice: wire.tool_choice, + stream: wire.stream, + store: wire.store, + include: wire.include, + temperature: wire.temperature, + top_p: wire.top_p, + max_output_tokens: wire.max_output_tokens, + truncation: wire.truncation, + metadata: wire.metadata, + parallel_tool_calls: wire.parallel_tool_calls, + cache_salt: wire.cache_salt, + context_management: wire.context_management, + }) + } +} + fn default_true() -> bool { true } @@ -252,8 +322,53 @@ pub struct ResponsePayload { pub incomplete_details: Option, pub error: Option, pub previous_response_id: Option, + #[serde( + rename = "conversation", + alias = "conversation_id", + serialize_with = "serialize_conversation_reference", + deserialize_with = "deserialize_conversation_reference" + )] pub conversation_id: Option, pub instructions: Option, + #[serde(default)] + pub parallel_tool_calls: bool, + pub temperature: Option, + #[serde(default)] + pub tool_choice: ToolChoice, + #[serde(default)] + pub tools: Vec, + pub top_p: Option, + pub truncation: Option, + pub metadata: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum ConversationReference { + Id(String), + Object { id: String }, +} + +#[allow(clippy::ref_option)] +fn serialize_conversation_reference(value: &Option, serializer: S) -> Result +where + S: serde::Serializer, +{ + match value { + Some(id) => serde_json::json!({"id": id}).serialize(serializer), + None => serializer.serialize_none(), + } +} + +fn deserialize_conversation_reference<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + Option::::deserialize(deserializer).map(|reference| { + reference.map(|reference| match reference { + ConversationReference::Id(id) | ConversationReference::Object { id } => id, + }) + }) } impl ResponsePayload { @@ -327,6 +442,85 @@ impl From for Vec { mod tests { use super::*; + #[test] + fn conversation_wire_field_uses_standard_name_and_accepts_legacy_alias() { + let standard: RequestPayload = serde_json::from_value(serde_json::json!({ + "model": "test-model", + "input": [], + "conversation": "conv_standard" + })) + .expect("standard conversation field"); + let legacy: RequestPayload = serde_json::from_value(serde_json::json!({ + "model": "test-model", + "input": [], + "conversation_id": "conv_legacy" + })) + .expect("legacy conversation field"); + let matching_aliases: RequestPayload = serde_json::from_value(serde_json::json!({ + "model": "test-model", + "input": [], + "conversation": "conv_same", + "conversation_id": "conv_same" + })) + .expect("matching conversation aliases"); + let conflicting_aliases = serde_json::from_value::(serde_json::json!({ + "model": "test-model", + "input": [], + "conversation": "conv_one", + "conversation_id": "conv_two" + })); + + assert_eq!(standard.conversation_id.as_deref(), Some("conv_standard")); + assert_eq!(legacy.conversation_id.as_deref(), Some("conv_legacy")); + assert_eq!(matching_aliases.conversation_id.as_deref(), Some("conv_same")); + assert!(conflicting_aliases.is_err()); + let serialized = serde_json::to_value(standard).expect("serialize standard conversation field"); + assert_eq!(serialized["conversation"], "conv_standard"); + assert!(serialized.get("conversation_id").is_none()); + } + + #[test] + fn response_conversation_serializes_as_standard_reference_and_accepts_legacy_shapes() { + let mut payload: ResponsePayload = serde_json::from_value(serde_json::json!({ + "id": "resp_test", + "object": "response", + "created_at": 0, + "model": "test-model", + "status": "completed", + "output": [], + "usage": null, + "incomplete_details": null, + "error": null, + "previous_response_id": null, + "conversation": {"id": "conv_standard"}, + "instructions": null + })) + .expect("standard response conversation reference"); + assert_eq!(payload.conversation_id.as_deref(), Some("conv_standard")); + + payload.conversation_id = Some("conv_serialized".to_owned()); + let serialized = serde_json::to_value(payload).expect("serialize response payload"); + assert_eq!(serialized["conversation"]["id"], "conv_serialized"); + assert!(serialized.get("conversation_id").is_none()); + + let legacy: ResponsePayload = serde_json::from_value(serde_json::json!({ + "id": "resp_legacy", + "object": "response", + "created_at": 0, + "model": "test-model", + "status": "completed", + "output": [], + "usage": null, + "incomplete_details": null, + "error": null, + "previous_response_id": null, + "conversation_id": "conv_legacy", + "instructions": null + })) + .expect("legacy response conversation reference"); + assert_eq!(legacy.conversation_id.as_deref(), Some("conv_legacy")); + } + #[test] fn compact_request_accepts_codex_compatibility_fields() { let request: CompactRequest = serde_json::from_value(serde_json::json!({ @@ -657,6 +851,13 @@ mod tests { previous_response_id: None, conversation_id: None, instructions: None, + parallel_tool_calls: true, + temperature: None, + tool_choice: ToolChoice::Auto, + tools: Vec::new(), + top_p: None, + truncation: None, + metadata: None, }; for (status, expected_type) in [ @@ -690,6 +891,13 @@ mod tests { previous_response_id: None, conversation_id: None, instructions: None, + parallel_tool_calls: true, + temperature: None, + tool_choice: ToolChoice::Auto, + tools: Vec::new(), + top_p: None, + truncation: None, + metadata: None, }; let chunk = payload.as_created_response_chunk(); diff --git a/crates/agentic-server/src/app.rs b/crates/agentic-server/src/app.rs index 89eb8947..55c69152 100644 --- a/crates/agentic-server/src/app.rs +++ b/crates/agentic-server/src/app.rs @@ -16,7 +16,9 @@ use agentic_core::proxy::ProxyState; use crate::auth::{ANTHROPIC_COUNT_TOKENS_PATH, ANTHROPIC_MESSAGES_PATH, OidcAuthenticator, require_oidc}; use crate::handler::{ - compact_response, conversations, count_tokens, health, messages, models, ready, responses, responses_ws_with_auth, + compact_response, conversations, count_tokens, create_items, delete_conversation, delete_item, health, list_items, + messages, models, ready, responses, responses_ws_with_auth, retrieve_conversation, retrieve_item, + update_conversation, }; #[derive(Clone, Default)] @@ -178,6 +180,20 @@ pub fn build_router_with_auth( let public_routes = Router::new().route("/health", get(health)).route("/ready", get(ready)); let protected_routes = Router::new() .route("/v1/conversations", post(conversations)) + .route( + "/v1/conversations/{conversation_id}", + get(retrieve_conversation) + .post(update_conversation) + .delete(delete_conversation), + ) + .route( + "/v1/conversations/{conversation_id}/items", + post(create_items).get(list_items), + ) + .route( + "/v1/conversations/{conversation_id}/items/{item_id}", + get(retrieve_item).delete(delete_item), + ) .route("/v1/models", get(models)) .route(ANTHROPIC_MESSAGES_PATH, post(messages)) .route(ANTHROPIC_COUNT_TOKENS_PATH, post(count_tokens)) diff --git a/crates/agentic-server/src/auth.rs b/crates/agentic-server/src/auth.rs index 490819a4..136ba153 100644 --- a/crates/agentic-server/src/auth.rs +++ b/crates/agentic-server/src/auth.rs @@ -325,6 +325,11 @@ impl AuthenticatedPrincipal { &self.subject } + #[must_use] + pub fn tenant_id(&self) -> String { + format!("{}:{}", self.issuer, self.subject) + } + #[must_use] pub fn expires_at(&self) -> u64 { self.expires_at diff --git a/crates/agentic-server/src/handler/common.rs b/crates/agentic-server/src/handler/common.rs index 7d83efd8..6d885b5b 100644 --- a/crates/agentic-server/src/handler/common.rs +++ b/crates/agentic-server/src/handler/common.rs @@ -67,13 +67,6 @@ pub(super) async fn read_json(body: Body) -> Result(&bytes).map_err(|error| executor_error_response(ExecutorError::from(error))) } -pub(super) fn extract_store(bytes: &[u8]) -> bool { - serde_json::from_slice::(bytes) - .ok() - .and_then(|j| j.get("store").and_then(serde_json::Value::as_bool)) - .unwrap_or(true) -} - pub(super) fn extract_bearer(headers: &HeaderMap, config_key: Option<&str>) -> Option { headers .get("authorization") diff --git a/crates/agentic-server/src/handler/http/conversations.rs b/crates/agentic-server/src/handler/http/conversations.rs index 26571319..4863de61 100644 --- a/crates/agentic-server/src/handler/http/conversations.rs +++ b/crates/agentic-server/src/handler/http/conversations.rs @@ -1,31 +1,316 @@ -use axum::extract::{Request, State}; +use axum::extract::{Extension, Path, Query, Request, State}; use axum::response::{IntoResponse, Response}; -use serde_json::json; +use serde::Deserialize; +use serde_json::{Value, json}; -use agentic_core::executor::{ExecutorError, create_conversation}; +use agentic_core::executor::ExecutorError; +use agentic_core::{InOutItem, InputItem}; -use super::super::common::{executor_error_response, extract_store, read_bytes}; +use super::super::common::{executor_error_response, read_bytes}; use crate::app::AppState; +use crate::auth::AuthenticatedPrincipal; -pub async fn conversations(State(state): State, req: Request) -> Response { +const MAX_ITEMS_PER_REQUEST: usize = 20; + +#[derive(Debug, Default, Deserialize)] +struct CreateConversationRequest { + #[serde(default)] + metadata: Option, + #[serde(default)] + items: Vec, +} + +#[derive(Debug, Deserialize)] +struct UpdateConversationRequest { + metadata: Option, +} + +#[derive(Debug, Deserialize)] +struct CreateItemsRequest { + items: Vec, +} + +#[derive(Debug, Default, Deserialize)] +pub struct ListItemsQuery { + after: Option, + limit: Option, + order: Option, + #[allow(dead_code)] + include: Option>, +} + +fn tenant_id(principal: Option>) -> Option { + principal.map(|Extension(principal)| principal.tenant_id()) +} + +#[allow(clippy::result_large_err)] +fn parse_body(bytes: &[u8]) -> Result { + serde_json::from_slice(bytes).map_err(|error| executor_error_response(ExecutorError::from(error))) +} + +#[allow(clippy::result_large_err)] +fn parse_input_items(values: Vec) -> Result, Response> { + if values.len() > MAX_ITEMS_PER_REQUEST { + return Err(executor_error_response(ExecutorError::InvalidRequest(format!( + "items must contain at most {MAX_ITEMS_PER_REQUEST} items" + )))); + } + values + .into_iter() + .map(|value| { + let item: InputItem = serde_json::from_value(value).map_err(|error| { + executor_error_response(ExecutorError::InvalidRequest(format!( + "invalid conversation item: {error}" + ))) + })?; + if matches!(item, InputItem::Unknown) { + return Err(executor_error_response(ExecutorError::InvalidRequest( + "unsupported conversation item type".to_owned(), + ))); + } + Ok(InOutItem::Input(item)) + }) + .collect() +} + +fn conversation_json(data: &agentic_core::ConversationData) -> Value { + let metadata = data + .metadata + .as_deref() + .and_then(agentic_core::utils::common::deserialize_from_str_opt::) + .unwrap_or_else(|| json!({})); + json!({ + "id": data.conversation_id, + "created_at": data.created_at, + "object": "conversation", + "metadata": metadata, + }) +} + +fn item_list_json(page: agentic_core::ConversationItemPage) -> Value { + let first_id = page.data.first().map(|item| item.id.clone()); + let last_id = page.data.last().map(|item| item.id.clone()); + json!({ + "object": "list", + "data": page.data.into_iter().map(|item| item.item).collect::>(), + "first_id": first_id, + "last_id": last_id, + "has_more": page.has_more, + }) +} + +pub async fn conversations( + State(state): State, + principal: Option>, + req: Request, +) -> Response { let (_, body) = req.into_parts(); let bytes = match read_bytes(body).await { - Ok(b) => b, - Err(e) => return e, + Ok(bytes) => bytes, + Err(error) => return error, + }; + let request = if bytes.is_empty() { + CreateConversationRequest::default() + } else { + match parse_body::(&bytes) { + Ok(request) => request, + Err(error) => return error, + } }; + let items = match parse_input_items(request.items) { + Ok(items) => items, + Err(error) => return error, + }; + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .create_with_items(tenant_id.as_deref(), request.metadata.as_ref(), items) + .await + { + Ok(data) => axum::Json(conversation_json(&data)).into_response(), + Err(error) => executor_error_response(error), + } +} - if !extract_store(&bytes) { - return executor_error_response(ExecutorError::InvalidRequest("conversations require store=true".into())); +pub async fn retrieve_conversation( + State(state): State, + principal: Option>, + Path(conversation_id): Path, +) -> Response { + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .get_by_id(&conversation_id, tenant_id.as_deref()) + .await + { + Ok(data) => axum::Json(conversation_json(&data)).into_response(), + Err(error) => executor_error_response(error), } +} - match create_conversation(&state.exec_ctx).await { - Ok(data) => axum::Json(json!({ - "id": data.conversation_id, - "created_at": data.created_at, - "object": "conversation", - "metadata": {} +pub async fn update_conversation( + State(state): State, + principal: Option>, + Path(conversation_id): Path, + req: Request, +) -> Response { + let (_, body) = req.into_parts(); + let bytes = match read_bytes(body).await { + Ok(bytes) => bytes, + Err(error) => return error, + }; + let request = match parse_body::(&bytes) { + Ok(request) => request, + Err(error) => return error, + }; + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .update_metadata(&conversation_id, tenant_id.as_deref(), request.metadata.as_ref()) + .await + { + Ok(data) => axum::Json(conversation_json(&data)).into_response(), + Err(error) => executor_error_response(error), + } +} + +pub async fn delete_conversation( + State(state): State, + principal: Option>, + Path(conversation_id): Path, +) -> Response { + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .delete(&conversation_id, tenant_id.as_deref()) + .await + { + Ok(()) => axum::Json(json!({ + "id": conversation_id, + "object": "conversation.deleted", + "deleted": true, + })) + .into_response(), + Err(error) => executor_error_response(error), + } +} + +pub async fn create_items( + State(state): State, + principal: Option>, + Path(conversation_id): Path, + req: Request, +) -> Response { + let (_, body) = req.into_parts(); + let bytes = match read_bytes(body).await { + Ok(bytes) => bytes, + Err(error) => return error, + }; + let request = match parse_body::(&bytes) { + Ok(request) => request, + Err(error) => return error, + }; + let items = match parse_input_items(request.items) { + Ok(items) => items, + Err(error) => return error, + }; + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .append_items(&conversation_id, tenant_id.as_deref(), items) + .await + { + Ok(items) => axum::Json(item_list_json(agentic_core::ConversationItemPage { + data: items, + has_more: false, })) .into_response(), - Err(e) => executor_error_response(e), + Err(error) => executor_error_response(error), + } +} + +pub async fn list_items( + State(state): State, + principal: Option>, + Path(conversation_id): Path, + Query(query): Query, +) -> Response { + let limit = query.limit.unwrap_or(20); + if !(1..=100).contains(&limit) { + return executor_error_response(ExecutorError::InvalidRequest( + "limit must be between 1 and 100".to_owned(), + )); + } + let descending = match query.order.as_deref().unwrap_or("desc") { + "asc" => false, + "desc" => true, + _ => { + return executor_error_response(ExecutorError::InvalidRequest( + "order must be either asc or desc".to_owned(), + )); + } + }; + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .list_items( + &conversation_id, + tenant_id.as_deref(), + query.after.as_deref(), + limit, + descending, + ) + .await + { + Ok(page) => axum::Json(item_list_json(page)).into_response(), + Err(error) => executor_error_response(error), + } +} + +pub async fn retrieve_item( + State(state): State, + principal: Option>, + Path((conversation_id, item_id)): Path<(String, String)>, +) -> Response { + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .get_item(&conversation_id, &item_id, tenant_id.as_deref()) + .await + { + Ok(item) => axum::Json(item.item).into_response(), + Err(error) => executor_error_response(error), + } +} + +pub async fn delete_item( + State(state): State, + principal: Option>, + Path((conversation_id, item_id)): Path<(String, String)>, +) -> Response { + let tenant_id = tenant_id(principal); + match state + .exec_ctx + .conv_handler + .delete_item(&conversation_id, &item_id, tenant_id.as_deref()) + .await + { + Ok(()) => match state + .exec_ctx + .conv_handler + .get_by_id(&conversation_id, tenant_id.as_deref()) + .await + { + Ok(data) => axum::Json(conversation_json(&data)).into_response(), + Err(error) => executor_error_response(error), + }, + Err(error) => executor_error_response(error), } } diff --git a/crates/agentic-server/src/handler/http/mod.rs b/crates/agentic-server/src/handler/http/mod.rs index e92526d7..f0ddfc20 100644 --- a/crates/agentic-server/src/handler/http/mod.rs +++ b/crates/agentic-server/src/handler/http/mod.rs @@ -3,7 +3,10 @@ mod messages; mod models; mod responses; -pub use conversations::conversations; +pub use conversations::{ + conversations, create_items, delete_conversation, delete_item, list_items, retrieve_conversation, retrieve_item, + update_conversation, +}; pub use messages::{count_tokens, messages}; pub use models::{health, models, ready}; pub use responses::{compact_response, responses}; diff --git a/crates/agentic-server/src/handler/http/responses.rs b/crates/agentic-server/src/handler/http/responses.rs index 02d3d648..cf18795e 100644 --- a/crates/agentic-server/src/handler/http/responses.rs +++ b/crates/agentic-server/src/handler/http/responses.rs @@ -1,4 +1,4 @@ -use axum::extract::{Request, State}; +use axum::extract::{Extension, Request, State}; use axum::http::request::Parts; use axum::response::{IntoResponse, Response}; use bytes::Bytes; @@ -7,7 +7,7 @@ use tracing::debug; use std::sync::Arc; -use agentic_core::executor::{ExecuteRequest, compact_response as execute_compaction}; +use agentic_core::executor::{ExecuteRequest, compact_response_for_tenant as execute_compaction}; use agentic_core::proxy::{ProxyRequest, proxy_request}; use agentic_core::types::request_response::{CompactRequest, RequestPayload}; use agentic_core::types::tools::ResponsesTool; @@ -16,6 +16,7 @@ use super::super::common::{ convert_response, executor_error_response, extract_bearer, read_and_parse, read_json, sse_response, }; use crate::app::AppState; +use crate::auth::AuthenticatedPrincipal; async fn proxy_responses(state: &AppState, parts: Parts, body: Bytes) -> Response { let proxy_req = ProxyRequest { @@ -26,10 +27,16 @@ async fn proxy_responses(state: &AppState, parts: Parts, body: Bytes) -> Respons convert_response(proxy_request(proxy_req, &state.proxy_state).await) } -async fn execute_responses(state: &AppState, parts: Parts, payload: RequestPayload) -> Response { +async fn execute_responses( + state: &AppState, + parts: Parts, + payload: RequestPayload, + tenant_id: Option, +) -> Response { let auth = extract_bearer(&parts.headers, state.openai_api_key.as_deref()); match ExecuteRequest::new(payload, Arc::clone(&state.exec_ctx)) .with_auth(auth) + .with_tenant_id(tenant_id) .run() .await { @@ -46,7 +53,11 @@ fn has_gateway_tools(payload: &RequestPayload) -> bool { .is_some_and(|tools| tools.iter().any(|tool| !matches!(tool, ResponsesTool::Function(_)))) } -pub async fn responses(State(state): State, req: Request) -> Response { +pub async fn responses( + State(state): State, + principal: Option>, + req: Request, +) -> Response { let (parts, body) = req.into_parts(); let (bytes, payload) = match read_and_parse(body).await { Ok(v) => v, @@ -74,21 +85,27 @@ pub async fn responses(State(state): State, req: Request) -> Response "routing HTTP responses request" ); + let tenant_id = principal.map(|Extension(principal)| principal.tenant_id()); if should_execute { - execute_responses(&state, parts, payload).await + execute_responses(&state, parts, payload, tenant_id).await } else { proxy_responses(&state, parts, bytes).await } } -pub async fn compact_response(State(state): State, req: Request) -> Response { +pub async fn compact_response( + State(state): State, + principal: Option>, + req: Request, +) -> Response { let (parts, body) = req.into_parts(); let request: CompactRequest = match read_json(body).await { Ok(request) => request, Err(response) => return response, }; let auth = extract_bearer(&parts.headers, state.openai_api_key.as_deref()); - match execute_compaction(request, state.exec_ctx.as_ref(), auth.as_deref()).await { + let tenant_id = principal.map(|Extension(principal)| principal.tenant_id()); + match execute_compaction(request, state.exec_ctx.as_ref(), auth.as_deref(), tenant_id).await { Ok(response) => axum::Json(response).into_response(), Err(error) => executor_error_response(error), } diff --git a/crates/agentic-server/src/handler/mod.rs b/crates/agentic-server/src/handler/mod.rs index d234ae56..c5c384ae 100644 --- a/crates/agentic-server/src/handler/mod.rs +++ b/crates/agentic-server/src/handler/mod.rs @@ -3,6 +3,9 @@ pub mod http; pub mod websocket; pub use common::{convert_response, executor_error_response}; -pub use http::{compact_response, conversations, count_tokens, health, messages, models, ready, responses}; +pub use http::{ + compact_response, conversations, count_tokens, create_items, delete_conversation, delete_item, health, list_items, + messages, models, ready, responses, retrieve_conversation, retrieve_item, update_conversation, +}; pub use websocket::responses_ws; pub(crate) use websocket::responses_ws_with_auth; diff --git a/crates/agentic-server/src/handler/websocket/responses.rs b/crates/agentic-server/src/handler/websocket/responses.rs index 6f5a424c..e4193534 100644 --- a/crates/agentic-server/src/handler/websocket/responses.rs +++ b/crates/agentic-server/src/handler/websocket/responses.rs @@ -13,9 +13,7 @@ use tokio_util::sync::CancellationToken; use tracing::{debug, warn}; use agentic_core::ResponseUsage; -use agentic_core::executor::{ - BoxStream, ExecuteRequest, ExecutorError, RequestContext, persist_turn, rehydrate_conversation, -}; +use agentic_core::executor::{BoxStream, ExecuteRequest, ExecutorError, RequestContext, persist_turn}; use agentic_core::types::request_response::RequestPayload; use agentic_core::utils::common::utcnow_str; @@ -117,6 +115,7 @@ async fn responses_ws_loop( &text, &shutdown_token, &mut queue, + principal.as_ref().map(AuthenticatedPrincipal::tenant_id), ) .await { @@ -196,6 +195,7 @@ where /// /// Any requests received from the client while the stream is active are /// pushed onto `queue` and processed by the caller in order after this returns. +#[allow(clippy::too_many_arguments)] async fn handle_ws_text( sender: &mut WsSender, receiver: &mut WsReceiver, @@ -204,6 +204,7 @@ async fn handle_ws_text( text: &str, shutdown_token: &CancellationToken, queue: &mut VecDeque, + tenant_id: Option, ) -> Result<(), WsError> { let value = serde_json::from_str::(text).map_err(WsError::InvalidJson)?; @@ -231,12 +232,13 @@ async fn handle_ws_text( if generate == Some(false) { debug!("handling non-generating websocket request locally"); - return complete_without_inference(sender, state, payload).await; + return complete_without_inference(sender, state, payload, tenant_id).await; } let auth = extract_bearer(headers, state.openai_api_key.as_deref()); let result = ExecuteRequest::new(payload, Arc::clone(&state.exec_ctx)) .with_auth(auth) + .with_tenant_id(tenant_id) .run() .await?; let Some(result) = keep_if_running(shutdown_token, result) else { @@ -256,8 +258,9 @@ async fn complete_without_inference( sender: &mut WsSender, state: &AppState, payload: RequestPayload, + tenant_id: Option, ) -> Result<(), WsError> { - let ctx = rehydrate_conversation(payload, &state.exec_ctx).await?; + let ctx = agentic_core::executor::rehydrate_conversation_for_tenant(payload, &state.exec_ctx, tenant_id).await?; let created_at = utcnow_str(); let created_event = empty_response_event(&ctx, created_at, "response.created", "in_progress", 0, None); let completed_event = empty_response_event( @@ -305,7 +308,7 @@ fn empty_response_event( "incomplete_details": null, "error": null, "previous_response_id": &ctx.original_request.previous_response_id, - "conversation_id": &ctx.conversation_id, + "conversation": ctx.conversation_id.as_ref().map(|id| serde_json::json!({"id": id})), "instructions": &ctx.enriched_request.instructions, }, }) From 5ab2fe54468d89357e7486925d3c00218ba04da4 Mon Sep 17 00:00:00 2001 From: prasanna Date: Wed, 5 Aug 2026 22:44:19 +0530 Subject: [PATCH 2/3] test coverage for conversion coverage Signed-off-by: prasanna --- README.md | 3 +- .../tests/postgres_storage_integration.rs | 65 ++++ .../stateful_conversation_integration.rs | 5 +- .../tests/stateful_responses_integration.rs | 1 + .../tests/storage_integration.rs | 358 ++++++++++++++++++ .../agentic-server-core/tests/support/mod.rs | 12 +- .../tests/tool_normalization_test.rs | 1 + crates/agentic-server/tests/common/mod.rs | 61 ++- .../tests/conversations_test.rs | 349 +++++++++++++++-- crates/agentic-server/tests/responses_test.rs | 275 +++++++++++++- .../tests/responses_websocket_test.rs | 70 +++- docs/api/index.md | 19 +- tests/openai_sdk_conformance.py | 125 ++++++ 13 files changed, 1283 insertions(+), 61 deletions(-) create mode 100644 tests/openai_sdk_conformance.py diff --git a/README.md b/README.md index db58d860..a9d61b69 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,8 @@ flowchart LR | --- | --- | --- | | `POST /v1/responses` | OpenAI-compatible Responses API with state, tools, and streaming | ✅ | | `GET /v1/responses` | WebSocket transport for the Responses API | ✅ | -| `POST /v1/conversations` | Conversation management | ✅ | +| `POST /v1/conversations` · `GET/POST/DELETE /v1/conversations/{id}` | Conversation management | ✅ | +| `GET/POST /v1/conversations/{id}/items` · `GET/DELETE .../{item_id}` | Conversation items and pagination | ✅ | | `GET /v1/models` | Model listing proxied from vLLM | ✅ | | `GET /health` · `GET /ready` | Liveness and readiness probes | ✅ | | Messages API | Anthropic-style stateful messages on shared primitives | 🚧 Planned | diff --git a/crates/agentic-server-core/tests/postgres_storage_integration.rs b/crates/agentic-server-core/tests/postgres_storage_integration.rs index 75f22ff0..1a6074f0 100644 --- a/crates/agentic-server-core/tests/postgres_storage_integration.rs +++ b/crates/agentic-server-core/tests/postgres_storage_integration.rs @@ -367,6 +367,71 @@ async fn postgres_concurrent_conversation_writes_have_contiguous_sequences() { second_pool.close().await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[ignore = "requires TEST_POSTGRES_URL pointing to an isolated PostgreSQL database"] +async fn postgres_concurrent_item_appends_have_contiguous_sequences() { + const WRITE_COUNT: usize = 12; + + let database_url = std::env::var("TEST_POSTGRES_URL").expect("TEST_POSTGRES_URL must be set"); + let postgres_config = PostgresConfig { + max_connections: u32::try_from(WRITE_COUNT).expect("test write count must fit in u32"), + acquire_timeout: Duration::from_secs(5), + lock_timeout: Duration::from_secs(1), + migration_timeout: Duration::from_secs(5), + statement_timeout: Duration::from_secs(5), + idle_timeout: Some(Duration::from_secs(30)), + max_lifetime: Some(Duration::from_secs(60)), + }; + let first_pool = create_pool_with_schema_and_configs(Some(&database_url), SqliteConfig::default(), postgres_config) + .await + .expect("initialize PostgreSQL database"); + let second_pool = create_pool_with_configs(Some(&database_url), SqliteConfig::default(), postgres_config) + .await + .expect("create independent PostgreSQL pool"); + let conversation_store = ConversationStore::new(first_pool.clone()); + let conversation = conversation_store.create().await.expect("create conversation"); + let barrier = Arc::new(Barrier::new(WRITE_COUNT + 1)); + let mut tasks = Vec::with_capacity(WRITE_COUNT); + + for index in 0..WRITE_COUNT { + let barrier = Arc::clone(&barrier); + let pool = if index % 2 == 0 { + first_pool.clone() + } else { + second_pool.clone() + }; + let conversation_id = conversation.conversation_id.clone(); + tasks.push(tokio::spawn(async move { + let store = ConversationStore::new(pool); + barrier.wait().await; + store + .append_items_for_tenant(&conversation_id, None, vec![input_item(&format!("append {index}"))]) + .await + })); + } + + barrier.wait().await; + for task in tasks { + task.await + .expect("join concurrent append") + .expect("append concurrent item"); + } + + let rows = + agentic_core::storage::models::item::get_items_by_conversation(&first_pool, &conversation.conversation_id) + .await + .expect("load concurrent conversation items"); + let sequences = rows + .iter() + .map(|row| row.seq.expect("conversation item sequence")) + .collect::>(); + let write_count = i64::try_from(WRITE_COUNT).expect("write count must fit in i64"); + assert_eq!(sequences, (0..write_count).collect::>()); + + first_pool.close().await; + second_pool.close().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[ignore = "requires TEST_POSTGRES_URL pointing to an isolated PostgreSQL database"] #[allow( diff --git a/crates/agentic-server-core/tests/stateful_conversation_integration.rs b/crates/agentic-server-core/tests/stateful_conversation_integration.rs index a6174eb5..c3de7509 100644 --- a/crates/agentic-server-core/tests/stateful_conversation_integration.rs +++ b/crates/agentic-server-core/tests/stateful_conversation_integration.rs @@ -15,7 +15,6 @@ use support::{ const DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/cassettes/text_only/conversation"); -/// Case 6 — two turns, non-streaming, via `conversation_id`. #[tokio::test] async fn test_two_turn_nonstreaming_conversation() { // Arrange @@ -56,7 +55,6 @@ async fn test_two_turn_nonstreaming_conversation() { assert_eq!(output_text(&p2), expected_text(t2)); } -/// Case 7 — two turns, streaming, via `conversation_id`. #[tokio::test] async fn test_two_turn_streaming_conversation() { // Arrange @@ -306,7 +304,7 @@ async fn test_multi_branch() { } #[tokio::test] -async fn test_store_false_with_conversation_id_hydrates_and_persists() { +async fn test_store_false_with_conversation_hydrates_and_persists() { let cassette = load_cassette(&format!("{DIR}/conv-store-false-followup-gpt-4o-nonstreaming.yaml")); let all: Vec<_> = cassette.turns.iter().collect(); let fixture = TestFixture::new(&all).await; @@ -334,7 +332,6 @@ async fn test_store_false_with_conversation_id_hydrates_and_persists() { assert_eq!(p1.status, "completed"); assert_eq!(output_text(&p1), expected_text(t1)); - // Turn 2: store=false but conversation_id passed — must rehydrate and persist locally let p2 = unwrap_blocking( execute( make_request( diff --git a/crates/agentic-server-core/tests/stateful_responses_integration.rs b/crates/agentic-server-core/tests/stateful_responses_integration.rs index f38966eb..16abe170 100644 --- a/crates/agentic-server-core/tests/stateful_responses_integration.rs +++ b/crates/agentic-server-core/tests/stateful_responses_integration.rs @@ -625,6 +625,7 @@ async fn test_previous_response_id_persists_inherited_tools_and_choice() { response_id: "resp_lookup".into(), conversation_id: None, conversation_version: None, + tenant_id: None, }; let stored = fixture diff --git a/crates/agentic-server-core/tests/storage_integration.rs b/crates/agentic-server-core/tests/storage_integration.rs index 62ee2388..90c65f8d 100644 --- a/crates/agentic-server-core/tests/storage_integration.rs +++ b/crates/agentic-server-core/tests/storage_integration.rs @@ -8,7 +8,9 @@ use agentic_core::storage::{ use agentic_core::storage::{ConversationVersion, InOutItem, StorageError}; use agentic_core::types::event::MessageStatus; use agentic_core::types::io::{InputItem, InputMessage, InputMessageContent, OutputItem, OutputMessage}; +use serde_json::json; use std::sync::Arc; +use tokio::sync::Barrier; use support::setup_pool; @@ -38,6 +40,329 @@ async fn test_conversation_store_create_and_get() { assert_eq!(retrieved.conversation_id, created.conversation_id); } +#[tokio::test] +async fn tenant_scoped_conversations_cannot_be_read_or_mutated_by_another_tenant() { + let pool = setup_pool().await; + let store = ConversationStore::new(pool); + let conversation = store + .create_with_items_for_tenant(Some("tenant_a"), None, vec![create_input_item("private")]) + .await + .expect("create tenant conversation"); + + assert!( + store + .get_for_tenant(&conversation.conversation_id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .append_items_for_tenant( + &conversation.conversation_id, + Some("tenant_b"), + vec![create_input_item("not allowed")], + ) + .await + .is_err() + ); + + let page = store + .list_items_for_tenant(&conversation.conversation_id, Some("tenant_a"), None, 20, false) + .await + .expect("list tenant conversation items"); + assert_eq!(page.data.len(), 1); +} + +#[tokio::test] +async fn conversation_item_crud_and_pagination_are_ordered_and_scoped() -> Result<(), Box> { + let pool = setup_pool().await; + let store = ConversationStore::new(pool); + let conversation = store + .create_with_items_for_tenant( + Some("tenant_crud"), + Some(&json!({"project": "storage"})), + vec![create_input_item("first"), create_input_item("second")], + ) + .await?; + + assert_eq!(conversation.metadata.as_deref(), Some(r#"{"project":"storage"}"#)); + let updated = store + .update_metadata_for_tenant( + &conversation.conversation_id, + Some("tenant_crud"), + Some(&json!({"project": "updated"})), + ) + .await?; + assert_eq!(updated.metadata.as_deref(), Some(r#"{"project":"updated"}"#)); + + let appended = store + .append_items_for_tenant( + &conversation.conversation_id, + Some("tenant_crud"), + vec![create_input_item("third")], + ) + .await?; + assert_eq!(appended.len(), 1); + + let ascending = store + .list_items_for_tenant(&conversation.conversation_id, Some("tenant_crud"), None, 1, false) + .await?; + assert_eq!(ascending.data.len(), 1); + assert_eq!(ascending.data[0].sequence, 0); + assert!(ascending.has_more); + + let after_first = store + .list_items_for_tenant( + &conversation.conversation_id, + Some("tenant_crud"), + Some(&ascending.data[0].id), + 1, + false, + ) + .await?; + assert_eq!(after_first.data[0].sequence, 1); + assert!(after_first.has_more); + + let descending = store + .list_items_for_tenant(&conversation.conversation_id, Some("tenant_crud"), None, 1, true) + .await?; + assert_eq!(descending.data[0].sequence, 2); + assert!(descending.has_more); + + let retrieved = store + .get_item_for_tenant(&conversation.conversation_id, &appended[0].id, Some("tenant_crud")) + .await?; + assert_eq!(retrieved.id, appended[0].id); + assert_eq!(retrieved.item["content"], "third"); + + store + .delete_item_for_tenant(&conversation.conversation_id, &appended[0].id, Some("tenant_crud")) + .await?; + assert!( + store + .get_item_for_tenant(&conversation.conversation_id, &appended[0].id, Some("tenant_crud")) + .await + .is_err() + ); + assert!( + store + .get_item_for_tenant(&conversation.conversation_id, "item_missing", Some("tenant_crud")) + .await + .is_err() + ); + + Ok(()) +} + +#[tokio::test] +#[allow(clippy::too_many_lines)] +async fn tenant_isolation_covers_conversation_item_and_response_paths() -> Result<(), Box> { + let pool = setup_pool().await; + let store = ConversationStore::new(Arc::clone(&pool)); + let conversation = store + .create_with_items_for_tenant( + Some("tenant_a"), + Some(&json!({"owner": "a"})), + vec![create_input_item("private")], + ) + .await?; + let item = store + .list_items_for_tenant(&conversation.conversation_id, Some("tenant_a"), None, 20, false) + .await? + .data + .into_iter() + .next() + .expect("tenant item"); + + assert!( + store + .get_or_create_for_tenant(&conversation.conversation_id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .get_for_tenant(&conversation.conversation_id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .update_metadata_for_tenant( + &conversation.conversation_id, + Some("tenant_b"), + Some(&json!({"owner": "b"})) + ) + .await + .is_err() + ); + assert!( + store + .delete_for_tenant(&conversation.conversation_id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .append_items_for_tenant( + &conversation.conversation_id, + Some("tenant_b"), + vec![create_input_item("denied")] + ) + .await + .is_err() + ); + assert!( + store + .list_items_for_tenant(&conversation.conversation_id, Some("tenant_b"), None, 20, false) + .await + .is_err() + ); + assert!( + store + .get_item_for_tenant(&conversation.conversation_id, &item.id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .delete_item_for_tenant(&conversation.conversation_id, &item.id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .rehydrate_snapshot_for_tenant(&conversation.conversation_id, Some("tenant_b")) + .await + .is_err() + ); + assert!( + store + .persist_if_version_for_tenant( + &conversation.conversation_id, + Some("tenant_b"), + ConversationVersion::LastSequence(0), + "resp_tenant_b", + None, + vec![create_input_item("denied")], + &ResponseMetadata::default(), + ) + .await + .is_err() + ); + + store + .persist_if_version_for_tenant( + &conversation.conversation_id, + Some("tenant_a"), + ConversationVersion::LastSequence(0), + "resp_tenant_a", + None, + vec![create_input_item("allowed")], + &ResponseMetadata::default(), + ) + .await?; + let response_store = ResponseStore::new(pool); + assert!( + response_store + .get_for_tenant("resp_tenant_a", Some("tenant_b")) + .await + .is_err() + ); + assert!( + response_store + .rehydrate_for_tenant("resp_tenant_a", Some("tenant_b")) + .await + .is_err() + ); + assert_eq!( + store + .get_for_tenant(&conversation.conversation_id, Some("tenant_a")) + .await? + .metadata + .as_deref(), + Some(r#"{"owner":"a"}"#) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn sqlite_concurrent_item_appends_allocate_contiguous_sequences() -> Result<(), Box> { + let db_path = std::env::temp_dir().join(format!("append_{}.db", uuid::Uuid::now_v7())); + let db_url = format!("sqlite://{}", db_path.display()); + let pool = create_pool_with_schema_and_sqlite_config( + Some(&db_url), + SqliteConfig { + max_connections: 8, + ..SqliteConfig::default() + }, + ) + .await?; + let store = ConversationStore::new(Arc::clone(&pool)); + let conversation = store.create().await?; + let writer_count = 16_usize; + let barrier = Arc::new(Barrier::new(writer_count)); + let mut tasks = Vec::with_capacity(writer_count); + for index in 0..writer_count { + let store = store.clone(); + let conversation_id = conversation.conversation_id.clone(); + let barrier = Arc::clone(&barrier); + tasks.push(tokio::spawn(async move { + barrier.wait().await; + store + .append_items_for_tenant( + &conversation_id, + None, + vec![create_input_item(&format!("item {index}"))], + ) + .await + })); + } + for task in tasks { + task.await??; + } + let page = store + .list_items_for_tenant(&conversation.conversation_id, None, None, 100, false) + .await?; + assert_eq!(page.data.len(), writer_count); + assert_eq!( + page.data.iter().map(|item| item.sequence).collect::>(), + (0..i64::try_from(writer_count)?).collect::>() + ); + pool.close().await; + let _ = std::fs::remove_file(db_path); + Ok(()) +} + +#[tokio::test] +async fn deleting_a_conversation_preserves_its_items() -> Result<(), Box> { + let pool = setup_pool().await; + let store = ConversationStore::new(Arc::clone(&pool)); + let conversation = store + .create_with_items_for_tenant(Some("tenant_a"), None, vec![create_input_item("preserved")]) + .await?; + + let item_count_before = sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM items WHERE tenant_id = $1") + .bind("tenant_a") + .fetch_one(pool.as_ref()) + .await?; + assert_eq!(item_count_before, 1); + + store + .delete_for_tenant(&conversation.conversation_id, Some("tenant_a")) + .await?; + + let item = sqlx::query_as::<_, (Option, Option)>( + "SELECT conversation_id, seq FROM items WHERE tenant_id = $1", + ) + .bind("tenant_a") + .fetch_one(pool.as_ref()) + .await?; + assert_eq!(item, (None, None)); + Ok(()) +} + #[tokio::test] async fn test_conversation_store_persist_and_rehydrate() { let pool = setup_pool().await; @@ -448,6 +773,39 @@ async fn test_response_store_with_previous_response() { assert_eq!(rehydrated.len(), 2); } +#[tokio::test] +async fn response_store_allocates_unique_ids_for_repeated_protocol_items() { + let pool = setup_pool().await; + let store = ResponseStore::new(pool); + let metadata = ResponseMetadata::default(); + let repeated_item = create_output_item("fc_search"); + + store + .persist( + "resp_repeated_protocol_ids", + None, + vec![repeated_item.clone(), repeated_item.clone(), repeated_item], + &metadata, + ) + .await + .expect("repeated protocol IDs must not collide in storage"); + + let response = store + .get("resp_repeated_protocol_ids") + .await + .expect("response should be stored"); + let unique_ids: std::collections::HashSet<&String> = response.history_item_ids.iter().collect(); + assert_eq!(response.history_item_ids.len(), 3); + assert_eq!(unique_ids.len(), 3); + + let rehydrated = store + .rehydrate("resp_repeated_protocol_ids") + .await + .expect("response should rehydrate"); + assert_eq!(rehydrated.len(), 3); + assert!(rehydrated.iter().all(|item| item == &create_output_item("fc_search"))); +} + // Edge case tests #[tokio::test] diff --git a/crates/agentic-server-core/tests/support/mod.rs b/crates/agentic-server-core/tests/support/mod.rs index 3a2754e1..e2a8b54f 100644 --- a/crates/agentic-server-core/tests/support/mod.rs +++ b/crates/agentic-server-core/tests/support/mod.rs @@ -323,7 +323,7 @@ pub fn text_response(text: &str) -> MockResponse { "incomplete_details": null, "error": null, "previous_response_id": null, - "conversation_id": null, + "conversation": null, "instructions": null }) .to_string(), @@ -355,15 +355,15 @@ pub fn make_request( store: bool, stream: bool, previous_response_id: Option, - conversation_id: Option, + conversation: Option, ) -> RequestPayload { - RequestPayload { + let request = RequestPayload { model: "test-model".to_string(), input: serde_json::from_value(serde_json::to_value(input).expect("serialize Responses input")) .expect("request should contain valid Responses input"), instructions: None, previous_response_id, - conversation_id, + conversation_id: conversation, tools: None, tool_choice: None, stream, @@ -377,7 +377,9 @@ pub fn make_request( parallel_tool_calls: None, cache_salt: None, context_management: None, - } + }; + serde_json::from_value(serde_json::to_value(request).expect("serialize standard Responses request")) + .expect("deserialize standard Responses request") } pub fn unwrap_blocking(result: Either) -> ResponsePayload { diff --git a/crates/agentic-server-core/tests/tool_normalization_test.rs b/crates/agentic-server-core/tests/tool_normalization_test.rs index f9de2aea..cd00f3e5 100644 --- a/crates/agentic-server-core/tests/tool_normalization_test.rs +++ b/crates/agentic-server-core/tests/tool_normalization_test.rs @@ -103,6 +103,7 @@ fn upstream_request_value(payload: RequestPayload, stream: bool) -> Value { response_id: "resp_test".to_string(), conversation_id: None, conversation_version: None, + tenant_id: None, }; let upstream_request = ctx .enriched_request diff --git a/crates/agentic-server/tests/common/mod.rs b/crates/agentic-server/tests/common/mod.rs index 17f8ff11..835593cf 100644 --- a/crates/agentic-server/tests/common/mod.rs +++ b/crates/agentic-server/tests/common/mod.rs @@ -1,3 +1,4 @@ +use std::path::PathBuf; use std::sync::Arc; use axum::Router; @@ -10,7 +11,7 @@ use tokio_util::sync::CancellationToken; use agentic_core::config::Config; use agentic_core::executor::{ConversationHandler, ExecutionContext, ResponseHandler}; use agentic_core::proxy::ProxyState; -use agentic_core::storage::{ConversationStore, ResponseStore}; +use agentic_core::storage::{ConversationStore, ResponseStore, create_pool_with_schema}; use agentic_server::app::{AppState, ServerConfig, WebSocketTracker, build_router}; pub fn test_config(llm_url: &str) -> Config { @@ -45,6 +46,64 @@ pub fn test_state(config: &Config) -> AppState { } } +#[allow(dead_code)] +struct TestDb { + path: PathBuf, +} + +#[allow(dead_code)] +impl TestDb { + fn new() -> Self { + Self { + path: std::env::temp_dir().join(format!("agentic_http_test_{}.db", uuid::Uuid::now_v7())), + } + } + + fn url(&self) -> String { + format!("sqlite://{}", self.path.display()) + } +} + +#[allow(dead_code)] +impl Drop for TestDb { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + let _ = std::fs::remove_file(self.path.with_extension("db-shm")); + let _ = std::fs::remove_file(self.path.with_extension("db-wal")); + } +} + +#[allow(dead_code)] +pub struct StorageBackedState { + pub state: AppState, + _db: TestDb, +} + +#[allow(dead_code)] +pub async fn storage_backed_state(llm_url: &str) -> StorageBackedState { + let db = TestDb::new(); + let pool = create_pool_with_schema(Some(&db.url())) + .await + .expect("create test database"); + let config = test_config(llm_url); + let exec_ctx = Arc::new(ExecutionContext::new( + ConversationHandler::new(ConversationStore::new(Arc::clone(&pool))), + ResponseHandler::new(ResponseStore::new(Arc::clone(&pool))), + Arc::new(reqwest::Client::new()), + config.llm_api_base.clone(), + )); + let proxy_state = ProxyState::new(config.clone()).expect("proxy state"); + let state = AppState { + proxy_state, + exec_ctx, + shutdown_token: CancellationToken::new(), + websocket_tracker: WebSocketTracker::default(), + llm_api_base: config.llm_api_base, + openai_api_key: config.openai_api_key, + }; + StorageBackedState { state, _db: db } +} + /// Spawn a minimal mock LLM that responds to `GET /health` with 200. pub async fn spawn_mock_llm() -> (String, tokio::task::JoinHandle<()>) { let app = Router::new().route("/health", get(|| async { StatusCode::OK.into_response() })); diff --git a/crates/agentic-server/tests/conversations_test.rs b/crates/agentic-server/tests/conversations_test.rs index 44204bbd..fd5f9dba 100644 --- a/crates/agentic-server/tests/conversations_test.rs +++ b/crates/agentic-server/tests/conversations_test.rs @@ -2,66 +2,331 @@ mod common; use http::StatusCode; -use common::{spawn_gateway, spawn_mock_llm, test_config, test_state}; +use common::{spawn_gateway, spawn_mock_llm, storage_backed_state, test_config, test_state}; #[tokio::test] -async fn test_conversations_store_false_returns_400() { - // Arrange - let (llm_url, _h1) = spawn_mock_llm().await; - let (gw_url, _h2) = spawn_gateway(test_state(&test_config(&llm_url))).await; +#[allow(clippy::too_many_lines)] +async fn test_conversations_create_retrieve_update_items_and_delete() { + let (llm_url, _llm) = spawn_mock_llm().await; + let fixture = storage_backed_state(&llm_url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); - // Act - let resp = reqwest::Client::new() - .post(format!("{gw_url}/v1/conversations")) - .json(&serde_json::json!({"store": false})) + let response = client + .post(format!("{gateway_url}/v1/conversations")) + .json(&serde_json::json!({ + "metadata": {"project": "agentic"}, + "items": [ + {"type": "message", "role": "user", "content": "hello"}, + { + "type": "function_call", + "id": "fc_initial", + "call_id": "call_initial", + "name": "lookup", + "arguments": "{}", + "status": "completed" + }, + {"type": "function_call_output", "call_id": "call_initial", "output": "done"}, + { + "type": "custom_tool_call", + "id": "ctc_initial", + "call_id": "call_custom", + "name": "echo", + "input": "hello", + "status": "completed" + }, + {"type": "custom_tool_call_output", "call_id": "call_custom", "output": {"value": "done"}} + ] + })) .send() .await - .unwrap(); + .expect("create conversation request"); + assert_eq!(response.status(), StatusCode::OK); + let conversation: serde_json::Value = response.json().await.expect("conversation response"); + let conversation_id = conversation["id"].as_str().expect("conversation ID"); + assert_eq!(conversation["object"], "conversation"); + assert_eq!(conversation["metadata"]["project"], "agentic"); - // Assert - assert_eq!(resp.status(), StatusCode::BAD_REQUEST); - let body: serde_json::Value = resp.json().await.unwrap(); - assert_eq!(body["error"]["code"], "invalid_request_error"); + let response = client + .get(format!("{gateway_url}/v1/conversations/{conversation_id}")) + .send() + .await + .expect("retrieve conversation request"); + assert_eq!(response.status(), StatusCode::OK); + let retrieved: serde_json::Value = response.json().await.expect("retrieved conversation"); + assert_eq!(retrieved["id"], conversation_id); + assert_eq!(retrieved["metadata"]["project"], "agentic"); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items?order=asc" + )) + .send() + .await + .expect("list initial items request"); + assert_eq!(response.status(), StatusCode::OK); + let initial_items: serde_json::Value = response.json().await.expect("initial item list"); + let initial_data = initial_items["data"].as_array().expect("initial item data"); + assert_eq!(initial_data.len(), 5); + assert_eq!( + initial_data + .iter() + .map(|item| item["type"].as_str().expect("item type")) + .collect::>(), + vec![ + "message", + "function_call", + "function_call_output", + "custom_tool_call", + "custom_tool_call_output" + ] + ); + let first_item_id = initial_items["data"][0]["id"].as_str().expect("initial item ID"); + let second_initial_item_id = initial_items["data"][1]["id"].as_str().expect("second initial item ID"); + assert_eq!(initial_items["data"][0]["type"], "message"); + assert!(!initial_items["has_more"].as_bool().expect("has_more")); + + let response = client + .post(format!("{gateway_url}/v1/conversations/{conversation_id}")) + .json(&serde_json::json!({"metadata": {"project": "updated"}})) + .send() + .await + .expect("update conversation request"); + assert_eq!(response.status(), StatusCode::OK); + let updated: serde_json::Value = response.json().await.expect("updated conversation"); + assert_eq!(updated["metadata"]["project"], "updated"); + + let response = client + .post(format!("{gateway_url}/v1/conversations/{conversation_id}/items")) + .json(&serde_json::json!({ + "items": [{"type": "message", "role": "user", "content": "follow up"}] + })) + .send() + .await + .expect("append items request"); + assert_eq!(response.status(), StatusCode::OK); + let appended: serde_json::Value = response.json().await.expect("appended item list"); + let appended_item_id = appended["data"][0]["id"].as_str().expect("appended item ID"); + assert_ne!(first_item_id, appended_item_id); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items?order=asc&limit=1" + )) + .send() + .await + .expect("list paginated items request"); + assert_eq!(response.status(), StatusCode::OK); + let page: serde_json::Value = response.json().await.expect("paginated item list"); + assert_eq!(page["data"].as_array().expect("page data").len(), 1); + assert_eq!(page["data"][0]["id"], first_item_id); + assert!(page["has_more"].as_bool().expect("page has_more")); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items?order=asc&after={first_item_id}&limit=1" + )) + .send() + .await + .expect("list items after cursor request"); + assert_eq!(response.status(), StatusCode::OK); + let after_page: serde_json::Value = response.json().await.expect("after cursor page"); + assert_eq!(after_page["data"].as_array().expect("after page data").len(), 1); + assert_eq!(after_page["data"][0]["id"], second_initial_item_id); + assert!(after_page["has_more"].as_bool().expect("after page has_more")); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items?limit=1" + )) + .send() + .await + .expect("default descending item page request"); + assert_eq!(response.status(), StatusCode::OK); + let descending_page: serde_json::Value = response.json().await.expect("descending page"); + assert_eq!(descending_page["data"][0]["id"], appended_item_id); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items/{appended_item_id}" + )) + .send() + .await + .expect("retrieve item request"); + assert_eq!(response.status(), StatusCode::OK); + let item: serde_json::Value = response.json().await.expect("retrieved item"); + assert_eq!(item["id"], appended_item_id); + + let response = client + .delete(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items/{appended_item_id}" + )) + .send() + .await + .expect("delete item request"); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .json::() + .await + .expect("delete item response")["id"], + conversation_id + ); + + let response = client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items/{appended_item_id}" + )) + .send() + .await + .expect("retrieve deleted item request"); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + + let response = client + .delete(format!("{gateway_url}/v1/conversations/{conversation_id}")) + .send() + .await + .expect("delete conversation request"); + assert_eq!(response.status(), StatusCode::OK); + let deleted: serde_json::Value = response.json().await.expect("deleted conversation"); + assert_eq!(deleted["object"], "conversation.deleted"); + assert!(deleted["deleted"].as_bool().expect("deleted flag")); + + let response = client + .get(format!("{gateway_url}/v1/conversations/{conversation_id}")) + .send() + .await + .expect("retrieve deleted conversation request"); + assert_eq!(response.status(), StatusCode::NOT_FOUND); } #[tokio::test] -async fn test_conversations_empty_body_defaults_store_true_reaches_executor() { - // Arrange — disabled store, so executor will error (not a 4xx) - let (llm_url, _h1) = spawn_mock_llm().await; - let (gw_url, _h2) = spawn_gateway(test_state(&test_config(&llm_url))).await; +async fn test_unknown_conversation_is_not_created_by_responses() { + let (llm_url, _llm) = spawn_mock_llm().await; + let fixture = storage_backed_state(&llm_url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; - // Act — empty JSON body; store defaults to true - let resp = reqwest::Client::new() - .post(format!("{gw_url}/v1/conversations")) - .header("Content-Type", "application/json") - .body("{}") + let response = reqwest::Client::new() + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": "hello"}], + "conversation": "conv_missing" + })) .send() .await - .unwrap(); + .expect("response request"); - // Assert — reached executor path (storage disabled → 5xx, not a 4xx rejection) - assert!( - !resp.status().is_client_error(), - "expected executor path, got client error" - ); + assert_eq!(response.status(), StatusCode::NOT_FOUND); + let body: serde_json::Value = response.json().await.expect("error body"); + assert_eq!(body["error"]["code"], "not_found"); +} + +#[tokio::test] +async fn test_conversation_and_item_not_found_errors_cover_public_routes() { + let (llm_url, _llm) = spawn_mock_llm().await; + let fixture = storage_backed_state(&llm_url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let missing_conversation = "conv_missing_public"; + + for response in [ + client + .get(format!("{gateway_url}/v1/conversations/{missing_conversation}")) + .send() + .await + .expect("retrieve missing conversation request"), + client + .post(format!("{gateway_url}/v1/conversations/{missing_conversation}")) + .json(&serde_json::json!({"metadata": {}})) + .send() + .await + .expect("update missing conversation request"), + client + .delete(format!("{gateway_url}/v1/conversations/{missing_conversation}")) + .send() + .await + .expect("delete missing conversation request"), + client + .get(format!("{gateway_url}/v1/conversations/{missing_conversation}/items")) + .send() + .await + .expect("list missing conversation items request"), + client + .post(format!("{gateway_url}/v1/conversations/{missing_conversation}/items")) + .json(&serde_json::json!({"items": [{"type": "message", "role": "user", "content": "x"}]})) + .send() + .await + .expect("append to missing conversation request"), + ] { + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!( + response.json::().await.expect("not found body")["error"]["code"], + "not_found" + ); + } + + let response = client + .post(format!("{gateway_url}/v1/conversations")) + .send() + .await + .expect("create conversation request"); + let conversation: serde_json::Value = response.json().await.expect("conversation response"); + let conversation_id = conversation["id"].as_str().expect("conversation ID"); + for response in [ + client + .get(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items/item_missing_public" + )) + .send() + .await + .expect("retrieve missing item request"), + client + .delete(format!( + "{gateway_url}/v1/conversations/{conversation_id}/items/item_missing_public" + )) + .send() + .await + .expect("delete missing item request"), + ] { + assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!( + response.json::().await.expect("item not found body")["error"]["code"], + "not_found" + ); + } } #[tokio::test] -async fn test_conversations_no_content_type_still_defaults_store_true() { - // Arrange - let (llm_url, _h1) = spawn_mock_llm().await; - let (gw_url, _h2) = spawn_gateway(test_state(&test_config(&llm_url))).await; +async fn test_conversations_without_storage_return_server_error_not_client_error() { + let (llm_url, _llm) = spawn_mock_llm().await; + let (gateway_url, _gateway) = spawn_gateway(test_state(&test_config(&llm_url))).await; - // Act — no body at all - let resp = reqwest::Client::new() - .post(format!("{gw_url}/v1/conversations")) + let response = reqwest::Client::new() + .post(format!("{gateway_url}/v1/conversations")) + .json(&serde_json::json!({"metadata": {}})) .send() .await - .unwrap(); + .expect("conversation request"); - // Assert — reached executor path (not a 4xx rejection) - assert!( - !resp.status().is_client_error(), - "expected executor path, got client error" - ); + assert!(!response.status().is_client_error()); +} + +#[tokio::test] +async fn test_conversations_empty_body_defaults_to_create_request() { + let (llm_url, _llm) = spawn_mock_llm().await; + let fixture = storage_backed_state(&llm_url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + + let response = reqwest::Client::new() + .post(format!("{gateway_url}/v1/conversations")) + .send() + .await + .expect("conversation request"); + + assert_eq!(response.status(), StatusCode::OK); + let conversation: serde_json::Value = response.json().await.expect("conversation response"); + assert_eq!(conversation["object"], "conversation"); + assert_eq!(conversation["metadata"], serde_json::json!({})); + assert!(conversation["id"].as_str().is_some()); } diff --git a/crates/agentic-server/tests/responses_test.rs b/crates/agentic-server/tests/responses_test.rs index 183c4762..4a863430 100644 --- a/crates/agentic-server/tests/responses_test.rs +++ b/crates/agentic-server/tests/responses_test.rs @@ -6,6 +6,7 @@ use axum::http::header; use axum::response::IntoResponse; use axum::routing::post; use http::StatusCode; +use std::collections::VecDeque; use std::convert::Infallible; use std::future::Future; use std::path::PathBuf; @@ -30,6 +31,8 @@ const COMPETING_RESPONSE_ID: &str = "resp_competing"; const CONFLICT_MESSAGE: &str = "conversation changed while the response was being generated; retry the request"; enum MockResponse { + StaticJson(String), + StaticSse(String), GatedJson { body: String, arrived: oneshot::Sender<()>, @@ -45,6 +48,7 @@ enum MockResponse { struct MockResponsesServer { url: String, + requests: Arc>>, handle: tokio::task::JoinHandle<()>, } @@ -101,17 +105,52 @@ impl MockResponsesServer { } async fn start(response: MockResponse) -> Self { + Self::start_with_responses(vec![response]).await + } + + async fn start_static_json(responses: Vec) -> Self { + Self::start_with_responses(responses.into_iter().map(MockResponse::StaticJson).collect()).await + } + + async fn start_static_sse(responses: Vec) -> Self { + Self::start_with_responses(responses.into_iter().map(MockResponse::StaticSse).collect()).await + } + + async fn start_with_responses(responses: Vec) -> Self { let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); - let response = Arc::new(Mutex::new(Some(response))); + let response = Arc::new(Mutex::new(VecDeque::from(responses))); let route_response = Arc::clone(&response); + let requests = Arc::new(Mutex::new(Vec::new())); + let route_requests = Arc::clone(&requests); let app = Router::new().route( "/v1/responses", - post(move || { + post(move |body: Bytes| { let response = Arc::clone(&route_response); + let requests = Arc::clone(&route_requests); async move { - let response = response.lock().await.take().expect("mock response already consumed"); + requests + .lock() + .await + .push(serde_json::from_slice(&body).expect("mock request body should be JSON")); + let response = response + .lock() + .await + .pop_front() + .expect("mock response queue exhausted"); match response { + MockResponse::StaticJson(body) => axum::response::Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "application/json") + .body(axum::body::Body::from(body)) + .unwrap() + .into_response(), + MockResponse::StaticSse(body) => axum::response::Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "text/event-stream; charset=utf-8") + .body(axum::body::Body::from(body)) + .unwrap() + .into_response(), MockResponse::GatedJson { body, arrived, release } => { let _ = arrived.send(()); let _ = release.await; @@ -147,9 +186,14 @@ impl MockResponsesServer { let handle = tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); Self { url: format!("http://{addr}"), + requests, handle, } } + + async fn request_bodies(&self) -> Vec { + self.requests.lock().await.clone() + } } impl Drop for MockResponsesServer { @@ -299,6 +343,227 @@ fn gated_sse_chunks() -> (String, String) { ) } +fn continuation_json_response(response_id: &str) -> String { + serde_json::json!({ + "id": response_id, + "object": "response", + "status": "completed", + "model": "test-model", + "output": [{ + "id": format!("msg_{response_id}"), + "type": "message", + "role": "assistant", + "status": "completed", + "content": [{"type": "output_text", "text": "ok", "annotations": []}] + }], + "created_at": 0 + }) + .to_string() +} + +fn continuation_sse_response(response_id: &str) -> String { + let created = serde_json::json!({ + "type": "response.created", + "sequence_number": 0, + "response": {"id": response_id, "status": "in_progress"} + }); + let added = serde_json::json!({ + "type": "response.output_item.added", + "sequence_number": 1, + "output_index": 0, + "item": {"id": format!("msg_{response_id}"), "type": "message"} + }); + let delta = serde_json::json!({ + "type": "response.output_text.delta", + "sequence_number": 2, + "item_id": format!("msg_{response_id}"), + "output_index": 0, + "content_index": 0, + "delta": "ok" + }); + let completed = serde_json::json!({ + "type": "response.completed", + "sequence_number": 3, + "response": {"id": response_id, "status": "completed", "usage": null} + }); + format!("data: {created}\n\ndata: {added}\n\ndata: {delta}\n\ndata: {completed}\n\ndata: [DONE]\n\n") +} + +#[tokio::test] +async fn http_json_conversation_continuation_uses_standard_field_and_history() { + let mock = MockResponsesServer::start_static_json(vec![ + continuation_json_response("resp_http_json_1"), + continuation_json_response("resp_http_json_2"), + ]) + .await; + let fixture = storage_backed_state(&mock.url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let conversation_id = create_conversation(&client, &gateway_url).await; + + for input in ["first", "second"] { + let response = client + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": input}], + "conversation": conversation_id, + "store": true, + "stream": false + })) + .send() + .await + .expect("blocking conversation request"); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.json::().await.expect("blocking response")["conversation"]["id"], + conversation_id + ); + } + + let requests = mock.request_bodies().await; + assert_eq!(requests.len(), 2); + assert!(requests[0].get("conversation").is_none()); + assert!(requests[1].get("conversation").is_none()); + assert_eq!(requests[1]["input"].as_array().expect("history input").len(), 3); +} + +#[tokio::test] +async fn http_accepts_legacy_conversation_id_input_alias() { + let mock = MockResponsesServer::start_static_json(vec![continuation_json_response("resp_http_legacy_alias")]).await; + let fixture = storage_backed_state(&mock.url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let conversation_id = create_conversation(&client, &gateway_url).await; + + let response = client + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": "legacy alias"}], + "conversation_id": conversation_id, + "store": true, + "stream": false + })) + .send() + .await + .expect("legacy conversation alias request"); + + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response + .json::() + .await + .expect("legacy alias response")["conversation"]["id"], + conversation_id + ); +} + +#[tokio::test] +async fn http_sse_conversation_continuation_uses_standard_field_and_history() { + let mock = MockResponsesServer::start_static_sse(vec![ + continuation_sse_response("resp_http_sse_1"), + continuation_sse_response("resp_http_sse_2"), + ]) + .await; + let fixture = storage_backed_state(&mock.url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let conversation_id = create_conversation(&client, &gateway_url).await; + + for input in ["first", "second"] { + let response = client + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": input}], + "conversation": conversation_id, + "store": true, + "stream": true + })) + .send() + .await + .expect("SSE conversation request"); + assert_eq!(response.status(), StatusCode::OK); + let events = sse_events(&response.text().await.expect("SSE response")); + assert_eq!(events.last().expect("terminal SSE event")["type"], "response.completed"); + } + + let requests = mock.request_bodies().await; + assert_eq!(requests.len(), 2); + assert!(requests[0].get("conversation").is_none()); + assert!(requests[1].get("conversation").is_none()); + assert_eq!(requests[1]["input"].as_array().expect("history input").len(), 3); +} + +#[tokio::test] +async fn http_store_false_with_existing_conversation_still_persists_context() { + let mock = MockResponsesServer::start_static_json(vec![ + continuation_json_response("resp_http_store_true"), + continuation_json_response("resp_http_store_false"), + ]) + .await; + let fixture = storage_backed_state(&mock.url).await; + let pool = Arc::clone(&fixture.pool); + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let conversation_id = create_conversation(&client, &gateway_url).await; + + for (input, store) in [("first", true), ("second", false)] { + let response = client + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": input}], + "conversation": conversation_id, + "store": store, + "stream": false + })) + .send() + .await + .expect("conversation store request"); + assert_eq!(response.status(), StatusCode::OK); + } + + let response_count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM responses") + .fetch_one(pool.as_ref()) + .await + .expect("response count"); + assert_eq!(response_count, 2); + let requests = mock.request_bodies().await; + assert_eq!(requests[1]["input"].as_array().expect("history input").len(), 3); +} + +#[tokio::test] +async fn http_rejects_conversation_and_previous_response_id_together() { + let (llm_url, _llm) = spawn_mock_llm().await; + let fixture = storage_backed_state(&llm_url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state).await; + let client = reqwest::Client::new(); + let conversation_id = create_conversation(&client, &gateway_url).await; + + for stream in [false, true] { + let response = client + .post(format!("{gateway_url}/v1/responses")) + .json(&serde_json::json!({ + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": "ambiguous"}], + "conversation": conversation_id, + "previous_response_id": "resp_ambiguous", + "store": true, + "stream": stream + })) + .send() + .await + .expect("ambiguous state request"); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + assert_eq!( + response.json::().await.expect("ambiguous error")["error"]["code"], + "invalid_request_error" + ); + } +} + async fn assert_only_competing_turn_persisted(pool: &Arc, conversation_id: &str) { let conversation_store = ConversationStore::new(Arc::clone(pool)); assert_eq!( @@ -610,7 +875,7 @@ async fn http_json_conversation_conflict_rejects_stale_turn_without_persisting_i .json(&serde_json::json!({ "model": "test-model", "input": [{"type": "message", "role": "user", "content": "stale turn"}], - "conversation_id": conversation_id, + "conversation": conversation_id, "store": true, "stream": false })) @@ -649,7 +914,7 @@ async fn http_sse_conversation_conflict_terminates_after_observable_delta_withou .json(&serde_json::json!({ "model": "test-model", "input": [{"type": "message", "role": "user", "content": "stale turn"}], - "conversation_id": conversation_id, + "conversation": conversation_id, "store": true, "stream": true })) diff --git a/crates/agentic-server/tests/responses_websocket_test.rs b/crates/agentic-server/tests/responses_websocket_test.rs index 3c0268be..3ded7442 100644 --- a/crates/agentic-server/tests/responses_websocket_test.rs +++ b/crates/agentic-server/tests/responses_websocket_test.rs @@ -789,6 +789,7 @@ async fn test_websocket_generate_false_prewarm_redacts_mcp_runtime_credentials() response_id: "resp_lookup".to_owned(), conversation_id: None, conversation_version: None, + tenant_id: None, }; let stored = fixture .state @@ -870,7 +871,7 @@ async fn websocket_conversation_conflict_ends_request_without_persisting_stale_t "type": "response.create", "model": "test-model", "input": [{"type": "message", "role": "user", "content": "stale turn"}], - "conversation_id": conversation_id, + "conversation": conversation_id, "store": true, "stream": true }), @@ -948,7 +949,7 @@ async fn websocket_generate_false_conversation_conflict_rejects_stale_local_comp "type": "response.create", "model": "test-model", "input": [{"type": "message", "role": "user", "content": "stale local turn"}], - "conversation_id": conversation_id, + "conversation": conversation_id, "generate": false, "store": true, "stream": true @@ -1552,6 +1553,71 @@ async fn test_websocket_continuation_rehydrates_previous_response() { assert_eq!(requests[1]["input"][2]["content"], "continue"); } +#[tokio::test] +async fn test_websocket_conversation_continuation_uses_standard_field_and_history() { + let mock = MockResponsesServer::start(vec![ + sse_response("resp_ws_conversation_1", "msg_ws_conversation_1", "HELLO"), + sse_response("resp_ws_conversation_2", "msg_ws_conversation_2", "WORLD"), + ]) + .await; + let fixture = storage_backed_state(&mock.url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state.clone()).await; + let conversation_id = create_conversation(&gateway_url).await; + let mut ws = connect_responses_ws(&gateway_url).await; + + for input in ["first", "second"] { + send_json( + &mut ws, + json!({ + "type": "response.create", + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": input}], + "conversation": conversation_id, + "store": true, + "stream": true + }), + ) + .await; + let events = recv_until_completed(&mut ws).await; + let response = &events.last().expect("completed response")["response"]; + assert_eq!(response["conversation"]["id"], conversation_id); + } + + let requests = mock.request_bodies().await; + assert_eq!(requests.len(), 2); + assert!(requests[0].get("conversation").is_none()); + assert!(requests[1].get("conversation").is_none()); + assert_eq!(requests[1]["input"].as_array().expect("history input").len(), 3); +} + +#[tokio::test] +async fn test_websocket_rejects_conversation_and_previous_response_id_together() { + let mock = MockResponsesServer::start(vec![]).await; + let fixture = storage_backed_state(&mock.url).await; + let (gateway_url, _gateway) = spawn_gateway(fixture.state.clone()).await; + let conversation_id = create_conversation(&gateway_url).await; + let mut ws = connect_responses_ws(&gateway_url).await; + + send_json( + &mut ws, + json!({ + "type": "response.create", + "model": "test-model", + "input": [{"type": "message", "role": "user", "content": "ambiguous"}], + "conversation": conversation_id, + "previous_response_id": "resp_ambiguous", + "store": true, + "stream": true + }), + ) + .await; + + let error = recv_json(&mut ws).await; + assert_eq!(error["type"], "error"); + assert_eq!(error["status"], StatusCode::BAD_REQUEST.as_u16()); + assert!(mock.request_bodies().await.is_empty()); +} + #[tokio::test] async fn test_websocket_unknown_previous_response_returns_error_event() { let mock = MockResponsesServer::start(vec![]).await; diff --git a/docs/api/index.md b/docs/api/index.md index dd57c5aa..861f82c3 100644 --- a/docs/api/index.md +++ b/docs/api/index.md @@ -51,10 +51,27 @@ For a complete GitHub-backed deployment example, see ### `POST /v1/responses` HTTP Responses requests use the OpenAI-compatible Responses shape. Requests -with `store=true`, `previous_response_id`, `conversation_id`, compaction input, +with `store=true`, `previous_response_id`, `conversation`, compaction input, or `context_management` run through the executor. Other stateless `store=false` requests are passed directly to the configured vLLM backend. +### Conversations + +The gateway exposes durable OpenAI-compatible conversation state: + +- `POST /v1/conversations` creates a conversation. Optional `metadata` and `items` are stored atomically. +- `GET /v1/conversations/{conversation_id}` retrieves a conversation. +- `POST /v1/conversations/{conversation_id}` replaces its metadata. +- `DELETE /v1/conversations/{conversation_id}` deletes the conversation. +- `GET` and `POST /v1/conversations/{conversation_id}/items` list or append items. Listing supports `after`, `limit`, and + `order=asc|desc` (the default is `desc`). +- `GET` and `DELETE /v1/conversations/{conversation_id}/items/{item_id}` retrieve or delete one item. + +Responses requests reference this state with the standard `conversation` field. The legacy `conversation_id` request +field remains accepted as a compatibility alias, but responses use `conversation`. + +When OIDC is enabled, conversations, items, and stored responses are isolated by the authenticated issuer and subject. + ### `POST /v1/responses/compact` Compacts direct input or a stored previous-response chain into a canonical diff --git a/tests/openai_sdk_conformance.py b/tests/openai_sdk_conformance.py new file mode 100644 index 00000000..c909efd5 --- /dev/null +++ b/tests/openai_sdk_conformance.py @@ -0,0 +1,125 @@ +"""Conformance checks using the public OpenAI Python Conversations and Responses clients. + +Run this against a running gateway configured with a model that can answer a short +Responses request. The script intentionally uses only public SDK resources. +""" + +from __future__ import annotations + +import os + +from openai import BadRequestError, NotFoundError, OpenAI + + +BASE_URL = os.environ.get("AGENTIC_API_BASE_URL", "http://127.0.0.1:8000/v1") +API_KEY = os.environ.get("OPENAI_API_KEY", "test-key") +MODEL = os.environ.get("OPENAI_TEST_MODEL", "test-model") + + +def assert_not_found(operation) -> None: + try: + operation() + except NotFoundError as error: + assert error.status_code == 404 + else: + raise AssertionError("operation unexpectedly succeeded") + + +def main() -> None: + client = OpenAI(base_url=BASE_URL, api_key=API_KEY, max_retries=0) + + conversation = client.conversations.create( + metadata={"suite": "openai-sdk"}, + items=[ + {"type": "message", "role": "user", "content": "hello"}, + { + "type": "function_call", + "id": "fc_sdk", + "call_id": "call_sdk", + "name": "lookup", + "arguments": "{}", + "status": "completed", + }, + {"type": "function_call_output", "call_id": "call_sdk", "output": "done"}, + { + "type": "custom_tool_call", + "id": "ctc_sdk", + "call_id": "custom_sdk", + "name": "echo", + "input": "hello", + "status": "completed", + }, + {"type": "custom_tool_call_output", "call_id": "custom_sdk", "output": "done"}, + ], + ) + assert conversation.object == "conversation" + assert conversation.metadata == {"suite": "openai-sdk"} + + retrieved = client.conversations.retrieve(conversation.id) + assert retrieved.id == conversation.id + updated = client.conversations.update(conversation.id, metadata={"suite": "openai-sdk-updated"}) + assert updated.metadata == {"suite": "openai-sdk-updated"} + + first_page = client.conversations.items.list(conversation.id, order="asc", limit=1) + assert len(first_page.data) == 1 + assert first_page.has_more + first_item_id = first_page.data[0].id + second_page = client.conversations.items.list( + conversation.id, + order="asc", + after=first_item_id, + limit=1, + ) + assert len(second_page.data) == 1 + assert second_page.data[0].id != first_item_id + descending_page = client.conversations.items.list(conversation.id, limit=1) + assert len(descending_page.data) == 1 + + appended = client.conversations.items.create( + conversation.id, + items=[{"type": "message", "role": "user", "content": "follow up"}], + ) + appended_item_id = appended.data[0].id + assert client.conversations.items.retrieve(appended_item_id, conversation_id=conversation.id).id == appended_item_id + assert client.conversations.items.delete(appended_item_id, conversation_id=conversation.id).id == conversation.id + assert_not_found(lambda: client.conversations.items.retrieve(appended_item_id, conversation_id=conversation.id)) + + response_conversation = client.conversations.create(metadata={"suite": "openai-sdk-responses"}) + first_response = client.responses.create( + model=MODEL, + input="first turn", + conversation=response_conversation.id, + store=True, + ) + assert first_response.conversation is not None + assert first_response.conversation.id == response_conversation.id + + second_response = client.responses.create( + model=MODEL, + input="second turn", + conversation=response_conversation.id, + store=False, + ) + assert second_response.conversation is not None + assert second_response.conversation.id == response_conversation.id + + try: + client.responses.create( + model=MODEL, + input="ambiguous state", + conversation=response_conversation.id, + previous_response_id=first_response.id, + store=True, + ) + except BadRequestError as error: + assert error.status_code == 400 + else: + raise AssertionError("conversation and previous_response_id should be rejected together") + + deleted = client.conversations.delete(conversation.id) + assert deleted.deleted + assert_not_found(lambda: client.conversations.retrieve(conversation.id)) + + +if __name__ == "__main__": + main() From 34df08390dc157cfef4d8c6d257b6c6c14d14668 Mon Sep 17 00:00:00 2001 From: prasanna Date: Thu, 6 Aug 2026 19:06:00 +0530 Subject: [PATCH 3/3] removed clippy ignore Signed-off-by: prasanna --- .../src/executor/accumulator.rs | 1 - .../src/executor/compaction.rs | 9 +- .../src/executor/modes/conversation.rs | 52 +++++++- .../src/executor/rehydrate.rs | 9 +- .../src/storage/conversation.rs | 80 ++++++++++- .../src/storage/models/conversation.rs | 39 +++++- .../src/storage/models/item.rs | 37 +++++- .../src/storage/models/response.rs | 12 +- .../src/storage/response.rs | 16 ++- .../src/types/request_response.rs | 4 - .../stateful_conversation_integration.rs | 66 ++++++++- tests/openai_sdk_conformance.py | 125 ------------------ 12 files changed, 299 insertions(+), 151 deletions(-) delete mode 100644 tests/openai_sdk_conformance.py diff --git a/crates/agentic-server-core/src/executor/accumulator.rs b/crates/agentic-server-core/src/executor/accumulator.rs index 4ec86786..3afac40c 100644 --- a/crates/agentic-server-core/src/executor/accumulator.rs +++ b/crates/agentic-server-core/src/executor/accumulator.rs @@ -533,7 +533,6 @@ impl ResponseAccumulator { previous_response_id: previous_response_id.map(str::to_string), conversation_id: self.conversation_id, instructions: instructions.map(str::to_string), - parallel_tool_calls: true, temperature: None, tool_choice: crate::types::io::ToolChoice::Auto, tools: Vec::new(), diff --git a/crates/agentic-server-core/src/executor/compaction.rs b/crates/agentic-server-core/src/executor/compaction.rs index 53d9d938..c2b3295a 100644 --- a/crates/agentic-server-core/src/executor/compaction.rs +++ b/crates/agentic-server-core/src/executor/compaction.rs @@ -1,5 +1,3 @@ -#![allow(clippy::missing_errors_doc)] - use crate::executor::error::{ExecutorError, ExecutorResult}; use crate::executor::request::{ExecutionContext, RequestContext}; use crate::executor::upstream::fetch_blocking_payload; @@ -268,6 +266,13 @@ pub async fn compact_response( compact_response_for_tenant(request, exec_ctx, auth, None).await } +/// Compacts resolved input while applying the supplied tenant scope to state lookup. +/// +/// # Errors +/// +/// Returns [`ExecutorError::InvalidRequest`] if neither input nor a previous response +/// ID is supplied. Propagates rehydration, inference, serialization, and compaction +/// validation failures from the executor pipeline. pub async fn compact_response_for_tenant( request: CompactRequest, exec_ctx: &ExecutionContext, diff --git a/crates/agentic-server-core/src/executor/modes/conversation.rs b/crates/agentic-server-core/src/executor/modes/conversation.rs index ae87205f..ac0e573e 100644 --- a/crates/agentic-server-core/src/executor/modes/conversation.rs +++ b/crates/agentic-server-core/src/executor/modes/conversation.rs @@ -1,7 +1,5 @@ //! Conversation storage handler — owns all conversation store operations. -#![allow(clippy::missing_errors_doc)] - use serde_json::Value; use crate::storage::{ @@ -63,6 +61,12 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Gets a conversation by ID within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// does not exist in the tenant scope, or the database query fails. pub async fn get_by_id(&self, conversation_id: &str, tenant_id: Option<&str>) -> ExecutorResult { self.store .get_for_tenant(conversation_id, tenant_id) @@ -78,6 +82,12 @@ impl ConversationHandler { self.store.create().await.map_err(ExecutorError::Storage) } + /// Creates a conversation with metadata and an initial ordered item sequence. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, metadata or an + /// item cannot be serialized, or the database transaction fails. pub async fn create_with_items( &self, tenant_id: Option<&str>, @@ -90,6 +100,13 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Replaces a conversation's metadata within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// does not exist in the tenant scope, metadata cannot be serialized, or the + /// database query fails. pub async fn update_metadata( &self, conversation_id: &str, @@ -102,6 +119,12 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Deletes a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// does not exist in the tenant scope, or the database transaction fails. pub async fn delete(&self, conversation_id: &str, tenant_id: Option<&str>) -> ExecutorResult<()> { self.store .delete_for_tenant(conversation_id, tenant_id) @@ -109,6 +132,13 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Appends items to a conversation in storage order. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// does not exist in the tenant scope, an item cannot be serialized, or the + /// database transaction fails. pub async fn append_items( &self, conversation_id: &str, @@ -121,6 +151,12 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Lists a page of conversation items in the requested order. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// or `after` item does not exist in the tenant scope, or the database query fails. pub async fn list_items( &self, conversation_id: &str, @@ -135,6 +171,12 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Gets one item from a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// or item does not exist in the tenant scope, or the database query fails. pub async fn get_item( &self, conversation_id: &str, @@ -147,6 +189,12 @@ impl ConversationHandler { .map_err(ExecutorError::Storage) } + /// Deletes one item from a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`ExecutorError::Storage`] if storage is disabled, the conversation + /// or item does not exist in the tenant scope, or the database query fails. pub async fn delete_item( &self, conversation_id: &str, diff --git a/crates/agentic-server-core/src/executor/rehydrate.rs b/crates/agentic-server-core/src/executor/rehydrate.rs index eb43645c..2cf6cf35 100644 --- a/crates/agentic-server-core/src/executor/rehydrate.rs +++ b/crates/agentic-server-core/src/executor/rehydrate.rs @@ -3,8 +3,6 @@ //! Builds a [`RequestContext`] by loading prior turns from storage and //! injecting them into the enriched request before it is forwarded to the LLM. -#![allow(clippy::missing_errors_doc)] - use crate::executor::error::{ExecutorError, ExecutorResult}; use crate::executor::request::{ExecutionContext, RequestContext}; use crate::storage::InOutItem; @@ -32,6 +30,13 @@ pub async fn rehydrate_conversation( rehydrate_conversation_for_tenant(request, exec_ctx, None).await } +/// Builds a request context and scopes stored-state lookup to `tenant_id`. +/// +/// # Errors +/// +/// Returns [`ExecutorError::InvalidRequest`] if both `conversation` and +/// `previous_response_id` are supplied. Propagates storage errors when the selected +/// state does not exist in the tenant scope or cannot be loaded. pub async fn rehydrate_conversation_for_tenant( request: RequestPayload, exec_ctx: &ExecutionContext, diff --git a/crates/agentic-server-core/src/storage/conversation.rs b/crates/agentic-server-core/src/storage/conversation.rs index fe8ce336..f9c79521 100644 --- a/crates/agentic-server-core/src/storage/conversation.rs +++ b/crates/agentic-server-core/src/storage/conversation.rs @@ -1,7 +1,5 @@ //! Conversation storage operations. -#![allow(clippy::missing_errors_doc)] - use std::convert::TryFrom; use std::sync::Arc; @@ -52,6 +50,13 @@ impl ConversationStore { self.create_with_items_for_tenant(None, None, Vec::new()).await } + /// Creates a tenant-scoped conversation with metadata and initial items. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled. Returns a + /// serialization error for invalid metadata or items, or a database error if + /// the transaction cannot be completed. pub async fn create_with_items_for_tenant( &self, tenant_id: Option<&str>, @@ -82,6 +87,12 @@ impl ConversationStore { self.get_or_create_for_tenant(conversation_id, None).await } + /// Gets or creates a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled or a database + /// error if the conversation cannot be queried or created. pub async fn get_or_create_for_tenant( &self, conversation_id: &str, @@ -101,6 +112,13 @@ impl ConversationStore { self.get_for_tenant(conversation_id, None).await } + /// Gets a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation is outside the tenant scope or absent, or a database + /// error if the query fails. pub async fn get_for_tenant( &self, conversation_id: &str, @@ -113,6 +131,13 @@ impl ConversationStore { Ok(row.into()) } + /// Replaces conversation metadata within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation is outside the tenant scope or absent, a serialization + /// error for invalid metadata, or a database error if the update fails. pub async fn update_metadata_for_tenant( &self, conversation_id: &str, @@ -127,6 +152,13 @@ impl ConversationStore { Ok(row.into()) } + /// Deletes a conversation within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation is outside the tenant scope or absent, or a database + /// error if the transaction fails. pub async fn delete_for_tenant(&self, conversation_id: &str, tenant_id: Option<&str>) -> StoreResult<()> { let pool = self.pool()?; if !conversation::delete(pool, conversation_id, tenant_id).await? { @@ -135,6 +167,13 @@ impl ConversationStore { Ok(()) } + /// Appends items atomically within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation is outside the tenant scope or absent, a serialization + /// error for invalid items, or a database error if locking or insertion fails. pub async fn append_items_for_tenant( &self, conversation_id: &str, @@ -157,6 +196,13 @@ impl ConversationStore { Ok(rows.into_iter().map(ConversationItemData::from).collect()) } + /// Lists a page of conversation items within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation or `after` item is absent from the tenant scope, or + /// a database error if the query fails. pub async fn list_items_for_tenant( &self, conversation_id: &str, @@ -191,6 +237,13 @@ impl ConversationStore { }) } + /// Gets one conversation item within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation or item is absent from the tenant scope, or a database + /// error if the query fails. pub async fn get_item_for_tenant( &self, conversation_id: &str, @@ -205,6 +258,13 @@ impl ConversationStore { Ok(row.into()) } + /// Deletes one conversation item within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation or item is absent from the tenant scope, or a database + /// error if the deletion fails. pub async fn delete_item_for_tenant( &self, conversation_id: &str, @@ -237,6 +297,14 @@ impl ConversationStore { self.rehydrate_snapshot_for_tenant(conversation_id, None).await } + /// Rehydrates a conversation and captures its storage version within a tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the conversation is absent from the tenant scope, + /// [`StorageError::InvalidConversationSequence`] for malformed stored ordering, + /// or a database error if the query fails. pub async fn rehydrate_snapshot_for_tenant( &self, conversation_id: &str, @@ -314,6 +382,14 @@ impl ConversationStore { } #[allow(clippy::too_many_arguments)] + /// Persists a tenant-scoped turn only when the stored version still matches. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, + /// [`StorageError::ConversationConflict`] if the version changed, a not-found + /// error if the conversation is absent from the tenant scope, a serialization + /// error for invalid items or metadata, or a database error if the transaction fails. pub async fn persist_if_version_for_tenant( &self, conversation_id: &str, diff --git a/crates/agentic-server-core/src/storage/models/conversation.rs b/crates/agentic-server-core/src/storage/models/conversation.rs index 2b93a06a..8bca7947 100644 --- a/crates/agentic-server-core/src/storage/models/conversation.rs +++ b/crates/agentic-server-core/src/storage/models/conversation.rs @@ -1,7 +1,5 @@ //! Conversation context and history. -#![allow(clippy::missing_errors_doc)] - use super::super::pool::{DbPool, DbResult, DbTransaction}; use crate::storage::backend::DatabaseBackend; use crate::utils::common::utcnow_str; @@ -32,6 +30,11 @@ pub async fn create(pool: &DbPool, id: &str) -> DbResult { create_with_metadata(pool, id, None, None).await } +/// Creates a conversation with tenant and metadata fields. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the transaction cannot be started, inserted, or committed. pub async fn create_with_metadata( pool: &DbPool, id: &str, @@ -44,6 +47,11 @@ pub async fn create_with_metadata( Ok(conversation) } +/// Creates a conversation with tenant and metadata fields in an existing transaction. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the insert fails. pub async fn create_with_metadata_in_tx( tx: &mut DbTransaction<'_>, id: &str, @@ -71,6 +79,12 @@ pub async fn get_or_create(pool: &DbPool, id: &str) -> DbResult { get_or_create_for_tenant(pool, id, None).await } +/// Gets or creates a conversation within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the upsert fails or an existing ID belongs to a +/// different tenant scope. pub async fn get_or_create_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult { let now = utcnow_str(); if let Some(tenant_id) = tenant_id { @@ -109,6 +123,11 @@ pub async fn get(pool: &DbPool, id: &str) -> DbResult> { get_for_tenant(pool, id, None).await } +/// Gets a conversation within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn get_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult> { match tenant_id { Some(tenant_id) => { @@ -127,6 +146,11 @@ pub async fn get_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> } } +/// Updates conversation metadata within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the update query fails. pub async fn update_metadata( pool: &DbPool, id: &str, @@ -158,6 +182,11 @@ pub async fn update_metadata( } } +/// Deletes a conversation and detaches its items within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the transaction cannot be started, queried, or committed. pub async fn delete(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult { let mut tx = pool.begin().await?; match tenant_id { @@ -216,6 +245,12 @@ pub async fn lock_in_tx(tx: &mut DbTransaction<'_>, id: &str) -> DbResult<()> { lock_in_tx_for_tenant(tx, id, None).await } +/// Locks a tenant-scoped conversation for the lifetime of an existing transaction. +/// +/// # Errors +/// +/// Returns [`sqlx::Error::RowNotFound`] if no matching conversation exists, or +/// another [`sqlx::Error`] if lock acquisition fails. pub async fn lock_in_tx_for_tenant(tx: &mut DbTransaction<'_>, id: &str, tenant_id: Option<&str>) -> DbResult<()> { if DatabaseBackend::from_connection(tx.as_mut()) == DatabaseBackend::Postgres { let locked_id = match tenant_id { diff --git a/crates/agentic-server-core/src/storage/models/item.rs b/crates/agentic-server-core/src/storage/models/item.rs index 6b31a5a7..b955f033 100644 --- a/crates/agentic-server-core/src/storage/models/item.rs +++ b/crates/agentic-server-core/src/storage/models/item.rs @@ -1,7 +1,5 @@ //! Conversation history item stored in the database. -#![allow(clippy::missing_errors_doc)] - use serde_json::Value; use std::fmt::Write; use tracing::warn; @@ -139,6 +137,11 @@ pub async fn create_in_tx( create_in_tx_with_tenant(tx, items, conversation_id, None).await } +/// Creates items in an existing transaction with optional conversation and tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if sequence allocation or a batched insert fails. pub async fn create_in_tx_with_tenant( tx: &mut DbTransaction<'_>, items: Vec<(String, String)>, @@ -237,6 +240,11 @@ pub async fn get_items(pool: &DbPool, ids: &[String]) -> DbResult> { get_items_for_tenant(pool, ids, None).await } +/// Gets items by ID within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if any batched query fails. pub async fn get_items_for_tenant(pool: &DbPool, ids: &[String], tenant_id: Option<&str>) -> DbResult> { if ids.is_empty() { return Ok(vec![]); @@ -271,6 +279,11 @@ pub async fn get_items_for_tenant(pool: &DbPool, ids: &[String], tenant_id: Opti Ok(rows) } +/// Gets one item from a conversation within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn get_item( pool: &DbPool, conversation_id: &str, @@ -298,10 +311,20 @@ pub async fn get_item( } } +/// Gets all items for an unscoped conversation in sequence order. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn get_items_by_conversation(pool: &DbPool, conversation_id: &str) -> DbResult> { get_items_by_conversation_for_tenant(pool, conversation_id, None).await } +/// Gets all items for a conversation and tenant scope in sequence order. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn get_items_by_conversation_for_tenant( pool: &DbPool, conversation_id: &str, @@ -328,6 +351,11 @@ pub async fn get_items_by_conversation_for_tenant( } } +/// Deletes a conversation item within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the deletion fails. pub async fn delete(pool: &DbPool, conversation_id: &str, item_id: &str, tenant_id: Option<&str>) -> DbResult { let result = match tenant_id { Some(tenant_id) => { @@ -360,6 +388,11 @@ pub async fn last_conversation_sequence_in_tx( last_conversation_sequence_in_tx_for_tenant(tx, conversation_id, None).await } +/// Returns the last item sequence for a tenant-scoped conversation in a transaction. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn last_conversation_sequence_in_tx_for_tenant( tx: &mut DbTransaction<'_>, conversation_id: &str, diff --git a/crates/agentic-server-core/src/storage/models/response.rs b/crates/agentic-server-core/src/storage/models/response.rs index fb37c57b..53f97429 100644 --- a/crates/agentic-server-core/src/storage/models/response.rs +++ b/crates/agentic-server-core/src/storage/models/response.rs @@ -1,7 +1,5 @@ //! LLM API response stored in the database. -#![allow(clippy::missing_errors_doc)] - use super::super::pool::{DbPool, DbResult, DbTransaction}; use crate::utils::common::{deserialize_from_string_opt, deserialize_from_string_opt_or_default, utcnow_str}; @@ -56,6 +54,11 @@ pub async fn create_in_tx( .await } +/// Creates a response in an existing transaction within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the insert fails. pub async fn create_in_tx_with_tenant( tx: &mut DbTransaction<'_>, id: &str, @@ -90,6 +93,11 @@ pub async fn get(pool: &DbPool, id: &str) -> DbResult> { get_for_tenant(pool, id, None).await } +/// Gets a response within the optional tenant scope. +/// +/// # Errors +/// +/// Returns [`sqlx::Error`] if the query fails. pub async fn get_for_tenant(pool: &DbPool, id: &str, tenant_id: Option<&str>) -> DbResult> { match tenant_id { Some(tenant_id) => { diff --git a/crates/agentic-server-core/src/storage/response.rs b/crates/agentic-server-core/src/storage/response.rs index 1701af5c..2616cc35 100644 --- a/crates/agentic-server-core/src/storage/response.rs +++ b/crates/agentic-server-core/src/storage/response.rs @@ -1,7 +1,5 @@ //! Response storage operations and queries. -#![allow(clippy::missing_errors_doc)] - use std::collections::HashMap; use std::convert::TryFrom; use std::sync::Arc; @@ -54,6 +52,13 @@ impl ResponseStore { self.get_for_tenant(response_id, None).await } + /// Retrieves a response within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the response is outside the tenant scope or absent, or a database + /// error if the query fails. pub async fn get_for_tenant(&self, response_id: &str, tenant_id: Option<&str>) -> StoreResult { let pool = self.pool()?; let row = response::get_for_tenant(pool, response_id, tenant_id) @@ -73,6 +78,13 @@ impl ResponseStore { self.rehydrate_for_tenant(response_id, None).await } + /// Rehydrates a response's ordered item history within the optional tenant scope. + /// + /// # Errors + /// + /// Returns [`StorageError::NotConfigured`] if storage is disabled, a not-found + /// error if the response is outside the tenant scope or absent, or a database + /// error if the response or its items cannot be queried. pub async fn rehydrate_for_tenant( &self, response_id: &str, diff --git a/crates/agentic-server-core/src/types/request_response.rs b/crates/agentic-server-core/src/types/request_response.rs index 28512cd9..3eb9c7ce 100644 --- a/crates/agentic-server-core/src/types/request_response.rs +++ b/crates/agentic-server-core/src/types/request_response.rs @@ -330,8 +330,6 @@ pub struct ResponsePayload { )] pub conversation_id: Option, pub instructions: Option, - #[serde(default)] - pub parallel_tool_calls: bool, pub temperature: Option, #[serde(default)] pub tool_choice: ToolChoice, @@ -851,7 +849,6 @@ mod tests { previous_response_id: None, conversation_id: None, instructions: None, - parallel_tool_calls: true, temperature: None, tool_choice: ToolChoice::Auto, tools: Vec::new(), @@ -891,7 +888,6 @@ mod tests { previous_response_id: None, conversation_id: None, instructions: None, - parallel_tool_calls: true, temperature: None, tool_choice: ToolChoice::Auto, tools: Vec::new(), diff --git a/crates/agentic-server-core/tests/stateful_conversation_integration.rs b/crates/agentic-server-core/tests/stateful_conversation_integration.rs index c3de7509..19de6e84 100644 --- a/crates/agentic-server-core/tests/stateful_conversation_integration.rs +++ b/crates/agentic-server-core/tests/stateful_conversation_integration.rs @@ -6,15 +6,69 @@ mod support; -use agentic_core::executor::{create_conversation, execute}; +use agentic_core::executor::{BoxStream, create_conversation, execute}; +use agentic_core::types::request_response::ResponsePayload; +use either::Either; +use futures::StreamExt; use std::sync::Arc; use support::{ - TestFixture, collect_stream, expected_text, load_cassette, make_request, output_text, request_input_texts, - responses_turns, unwrap_blocking, + TestFixture, Turn, expected_text, load_cassette, make_request, output_text, request_input_texts, responses_turns, + unwrap_blocking, }; const DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/cassettes/text_only/conversation"); +fn recorded_event_types(turn: &Turn) -> Vec { + turn.response + .sse + .as_ref() + .expect("streaming cassette should contain SSE") + .iter() + .flat_map(|entry| entry.lines()) + .filter_map(|line| line.strip_prefix("data: ")) + .filter(|data| *data != "[DONE]") + .map(|data| serde_json::from_str::(data).expect("cassette SSE data should be valid JSON")) + .filter_map(|event| event["type"].as_str().map(str::to_owned)) + .collect() +} + +async fn collect_stream_lifecycle(result: Either) -> (ResponsePayload, Vec) { + let mut stream = match result { + Either::Right(stream) => stream, + Either::Left(_) => panic!("expected streaming response, got blocking"), + }; + let mut event_types = Vec::new(); + let mut terminal = None; + + while let Some(chunk) = stream.next().await { + for line in chunk.lines() { + let Some(data) = line.strip_prefix("data: ") else { + continue; + }; + if data == "[DONE]" { + continue; + } + let Ok(mut event) = serde_json::from_str::(data) else { + continue; + }; + let Some(event_type) = event["type"].as_str() else { + continue; + }; + event_types.push(event_type.to_owned()); + if event_type == "response.completed" + && let Some(response) = event.get_mut("response") + { + terminal = serde_json::from_value(response.take()).ok(); + } + } + } + + ( + terminal.expect("stream should contain a response.completed payload"), + event_types, + ) +} + #[tokio::test] async fn test_two_turn_nonstreaming_conversation() { // Arrange @@ -68,7 +122,7 @@ async fn test_two_turn_streaming_conversation() { let conv_id = create_conversation(ctx).await.expect("create conv").conversation_id; // Act - let p1 = collect_stream( + let (p1, p1_event_types) = collect_stream_lifecycle( execute( make_request(&t1.request.body.input, true, true, None, Some(conv_id.clone())), Arc::clone(ctx), @@ -77,7 +131,7 @@ async fn test_two_turn_streaming_conversation() { .expect("t1"), ) .await; - let p2 = collect_stream( + let (p2, p2_event_types) = collect_stream_lifecycle( execute( make_request(&t2.request.body.input, true, true, None, Some(conv_id)), Arc::clone(ctx), @@ -91,9 +145,11 @@ async fn test_two_turn_streaming_conversation() { assert!(p1.id.starts_with("resp_")); assert_eq!(p1.status, "completed"); assert_eq!(output_text(&p1), expected_text(t1)); + assert_eq!(p1_event_types, recorded_event_types(t1)); assert_ne!(p2.id, p1.id); assert_eq!(p2.status, "completed"); assert_eq!(output_text(&p2), expected_text(t2)); + assert_eq!(p2_event_types, recorded_event_types(t2)); } /// Case 8 — two independent conversations must not share context. diff --git a/tests/openai_sdk_conformance.py b/tests/openai_sdk_conformance.py deleted file mode 100644 index c909efd5..00000000 --- a/tests/openai_sdk_conformance.py +++ /dev/null @@ -1,125 +0,0 @@ -"""Conformance checks using the public OpenAI Python Conversations and Responses clients. - -Run this against a running gateway configured with a model that can answer a short -Responses request. The script intentionally uses only public SDK resources. -""" - -from __future__ import annotations - -import os - -from openai import BadRequestError, NotFoundError, OpenAI - - -BASE_URL = os.environ.get("AGENTIC_API_BASE_URL", "http://127.0.0.1:8000/v1") -API_KEY = os.environ.get("OPENAI_API_KEY", "test-key") -MODEL = os.environ.get("OPENAI_TEST_MODEL", "test-model") - - -def assert_not_found(operation) -> None: - try: - operation() - except NotFoundError as error: - assert error.status_code == 404 - else: - raise AssertionError("operation unexpectedly succeeded") - - -def main() -> None: - client = OpenAI(base_url=BASE_URL, api_key=API_KEY, max_retries=0) - - conversation = client.conversations.create( - metadata={"suite": "openai-sdk"}, - items=[ - {"type": "message", "role": "user", "content": "hello"}, - { - "type": "function_call", - "id": "fc_sdk", - "call_id": "call_sdk", - "name": "lookup", - "arguments": "{}", - "status": "completed", - }, - {"type": "function_call_output", "call_id": "call_sdk", "output": "done"}, - { - "type": "custom_tool_call", - "id": "ctc_sdk", - "call_id": "custom_sdk", - "name": "echo", - "input": "hello", - "status": "completed", - }, - {"type": "custom_tool_call_output", "call_id": "custom_sdk", "output": "done"}, - ], - ) - assert conversation.object == "conversation" - assert conversation.metadata == {"suite": "openai-sdk"} - - retrieved = client.conversations.retrieve(conversation.id) - assert retrieved.id == conversation.id - updated = client.conversations.update(conversation.id, metadata={"suite": "openai-sdk-updated"}) - assert updated.metadata == {"suite": "openai-sdk-updated"} - - first_page = client.conversations.items.list(conversation.id, order="asc", limit=1) - assert len(first_page.data) == 1 - assert first_page.has_more - first_item_id = first_page.data[0].id - second_page = client.conversations.items.list( - conversation.id, - order="asc", - after=first_item_id, - limit=1, - ) - assert len(second_page.data) == 1 - assert second_page.data[0].id != first_item_id - descending_page = client.conversations.items.list(conversation.id, limit=1) - assert len(descending_page.data) == 1 - - appended = client.conversations.items.create( - conversation.id, - items=[{"type": "message", "role": "user", "content": "follow up"}], - ) - appended_item_id = appended.data[0].id - assert client.conversations.items.retrieve(appended_item_id, conversation_id=conversation.id).id == appended_item_id - assert client.conversations.items.delete(appended_item_id, conversation_id=conversation.id).id == conversation.id - assert_not_found(lambda: client.conversations.items.retrieve(appended_item_id, conversation_id=conversation.id)) - - response_conversation = client.conversations.create(metadata={"suite": "openai-sdk-responses"}) - first_response = client.responses.create( - model=MODEL, - input="first turn", - conversation=response_conversation.id, - store=True, - ) - assert first_response.conversation is not None - assert first_response.conversation.id == response_conversation.id - - second_response = client.responses.create( - model=MODEL, - input="second turn", - conversation=response_conversation.id, - store=False, - ) - assert second_response.conversation is not None - assert second_response.conversation.id == response_conversation.id - - try: - client.responses.create( - model=MODEL, - input="ambiguous state", - conversation=response_conversation.id, - previous_response_id=first_response.id, - store=True, - ) - except BadRequestError as error: - assert error.status_code == 400 - else: - raise AssertionError("conversation and previous_response_id should be rejected together") - - deleted = client.conversations.delete(conversation.id) - assert deleted.deleted - assert_not_found(lambda: client.conversations.retrieve(conversation.id)) - - -if __name__ == "__main__": - main()