chore: use a Conversation type (#3735)
This commit is contained in:
@@ -5,7 +5,8 @@
|
||||
// - Backup creation
|
||||
// Additional debug logging can be added if needed for troubleshooting.
|
||||
|
||||
use crate::message::Message;
|
||||
use crate::conversation::message::Message;
|
||||
use crate::conversation::Conversation;
|
||||
use crate::providers::base::Provider;
|
||||
use crate::utils::safe_truncate;
|
||||
use anyhow::Result;
|
||||
@@ -399,7 +400,7 @@ pub fn generate_session_id() -> String {
|
||||
/// Security features:
|
||||
/// - Validates file paths to prevent directory traversal
|
||||
/// - Includes all security limits from read_messages_with_truncation
|
||||
pub fn read_messages(session_file: &Path) -> Result<Vec<Message>> {
|
||||
pub fn read_messages(session_file: &Path) -> Result<Conversation> {
|
||||
// Validate the path for security
|
||||
let secure_path = get_path(Identifier::Path(session_file.to_path_buf()))?;
|
||||
|
||||
@@ -428,7 +429,7 @@ pub fn read_messages(session_file: &Path) -> Result<Vec<Message>> {
|
||||
pub fn read_messages_with_truncation(
|
||||
session_file: &Path,
|
||||
max_content_size: Option<usize>,
|
||||
) -> Result<Vec<Message>> {
|
||||
) -> Result<Conversation> {
|
||||
// Security check: file size limit
|
||||
if session_file.exists() {
|
||||
let metadata = fs::metadata(session_file)?;
|
||||
@@ -626,7 +627,7 @@ pub fn read_messages_with_truncation(
|
||||
}
|
||||
}
|
||||
|
||||
Ok(messages)
|
||||
Ok(Conversation::new_unvalidated(messages))
|
||||
}
|
||||
|
||||
/// Parse a message from JSON string with optional content truncation
|
||||
@@ -685,7 +686,7 @@ fn parse_message_with_truncation(
|
||||
|
||||
/// Truncate content within a message in place
|
||||
fn truncate_message_content_in_place(message: &mut Message, max_content_size: usize) {
|
||||
use crate::message::MessageContent;
|
||||
use crate::conversation::message::MessageContent;
|
||||
use rmcp::model::{RawContent, ResourceContents};
|
||||
|
||||
for content in &mut message.content {
|
||||
@@ -1051,7 +1052,7 @@ pub fn read_metadata(session_file: &Path) -> Result<SessionMetadata> {
|
||||
/// - Validates file paths to prevent directory traversal
|
||||
pub async fn persist_messages(
|
||||
session_file: &Path,
|
||||
messages: &[Message],
|
||||
messages: &Conversation,
|
||||
provider: Option<Arc<dyn Provider>>,
|
||||
working_dir: Option<PathBuf>,
|
||||
) -> Result<()> {
|
||||
@@ -1069,7 +1070,7 @@ pub async fn persist_messages(
|
||||
/// - Uses atomic file operations via save_messages_with_metadata
|
||||
pub async fn persist_messages_with_schedule_id(
|
||||
session_file: &Path,
|
||||
messages: &[Message],
|
||||
messages: &Conversation,
|
||||
provider: Option<Arc<dyn Provider>>,
|
||||
schedule_id: Option<String>,
|
||||
working_dir: Option<PathBuf>,
|
||||
@@ -1144,7 +1145,7 @@ pub async fn persist_messages_with_schedule_id(
|
||||
pub fn save_messages_with_metadata(
|
||||
session_file: &Path,
|
||||
metadata: &SessionMetadata,
|
||||
messages: &[Message],
|
||||
messages: &Conversation,
|
||||
) -> Result<()> {
|
||||
use fs2::FileExt;
|
||||
|
||||
@@ -1257,7 +1258,7 @@ pub fn save_messages_with_metadata(
|
||||
/// of the session based on the conversation history.
|
||||
pub async fn generate_description(
|
||||
session_file: &Path,
|
||||
messages: &[Message],
|
||||
messages: &Conversation,
|
||||
provider: Arc<dyn Provider>,
|
||||
working_dir: Option<PathBuf>,
|
||||
) -> Result<()> {
|
||||
@@ -1275,7 +1276,7 @@ pub async fn generate_description(
|
||||
/// - Uses secure file operations for saving
|
||||
pub async fn generate_description_with_schedule_id(
|
||||
session_file: &Path,
|
||||
messages: &[Message],
|
||||
messages: &Conversation,
|
||||
provider: Arc<dyn Provider>,
|
||||
schedule_id: Option<String>,
|
||||
working_dir: Option<PathBuf>,
|
||||
@@ -1346,7 +1347,7 @@ pub async fn update_metadata(session_file: &Path, metadata: &SessionMetadata) ->
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::message::MessageContent;
|
||||
use crate::conversation::message::{Message, MessageContent};
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
@@ -1428,10 +1429,10 @@ mod tests {
|
||||
let file_path = dir.path().join("test.jsonl");
|
||||
|
||||
// Create some test messages
|
||||
let messages = vec![
|
||||
let messages = Conversation::new_unvalidated(vec![
|
||||
Message::user().with_text("Hello"),
|
||||
Message::assistant().with_text("Hi there"),
|
||||
];
|
||||
]);
|
||||
|
||||
// Write messages
|
||||
persist_messages(&file_path, &messages, None, None).await?;
|
||||
@@ -1535,7 +1536,7 @@ mod tests {
|
||||
"}]",
|
||||
];
|
||||
|
||||
let mut messages = Vec::new();
|
||||
let mut messages = Conversation::empty();
|
||||
for text in special_chars {
|
||||
messages.push(Message::user().with_text(text));
|
||||
messages.push(Message::assistant().with_text(text));
|
||||
@@ -1601,10 +1602,10 @@ mod tests {
|
||||
|
||||
// Create a message with content larger than the 50KB truncation limit
|
||||
let very_large_text = "A".repeat(100_000); // 100KB of text
|
||||
let messages = vec![
|
||||
let messages = Conversation::new_unvalidated(vec![
|
||||
Message::user().with_text(&very_large_text),
|
||||
Message::assistant().with_text("Small response"),
|
||||
];
|
||||
]);
|
||||
|
||||
// Write messages
|
||||
persist_messages(&file_path, &messages, None, None).await?;
|
||||
@@ -1615,7 +1616,9 @@ mod tests {
|
||||
assert_eq!(messages.len(), read_messages.len());
|
||||
|
||||
// First message should be truncated
|
||||
if let Some(MessageContent::Text(read_text)) = read_messages[0].content.first() {
|
||||
if let Some(MessageContent::Text(read_text)) =
|
||||
read_messages.first().unwrap().content.first()
|
||||
{
|
||||
assert!(
|
||||
read_text.text.len() < very_large_text.len(),
|
||||
"Content should be truncated"
|
||||
@@ -1635,7 +1638,7 @@ mod tests {
|
||||
}
|
||||
|
||||
// Second message should be unchanged
|
||||
if let Some(MessageContent::Text(read_text)) = read_messages[1].content.first() {
|
||||
if let Some(MessageContent::Text(read_text)) = read_messages.messages()[1].content.first() {
|
||||
assert_eq!(read_text.text, "Small response");
|
||||
} else {
|
||||
panic!("Expected text content in second message");
|
||||
@@ -1652,7 +1655,7 @@ mod tests {
|
||||
let mut metadata = SessionMetadata::default();
|
||||
metadata.description = "Description with\nnewline and \"quotes\" and 🦆".to_string();
|
||||
|
||||
let messages = vec![Message::user().with_text("test")];
|
||||
let messages = Conversation::new_unvalidated(vec![Message::user().with_text("test")]);
|
||||
|
||||
// Write with special metadata
|
||||
save_messages_with_metadata(&file_path, &metadata, &messages)?;
|
||||
@@ -1679,7 +1682,7 @@ mod tests {
|
||||
assert_eq!(metadata.working_dir, get_home_dir());
|
||||
|
||||
// Test deserialization of invalid directory
|
||||
let messages = vec![Message::user().with_text("test")];
|
||||
let messages = Conversation::new_unvalidated(vec![Message::user().with_text("test")]);
|
||||
save_messages_with_metadata(&file_path, &metadata, &messages)?;
|
||||
|
||||
// Modify the file to include invalid directory
|
||||
@@ -1709,7 +1712,8 @@ mod tests {
|
||||
let working_dir_path = working_dir.path().to_path_buf();
|
||||
|
||||
// Create messages
|
||||
let messages = vec![Message::user().with_text("test message")];
|
||||
let messages =
|
||||
Conversation::new_unvalidated(vec![Message::user().with_text("test message")]);
|
||||
|
||||
// Use persist_messages_with_schedule_id to set working dir
|
||||
persist_messages_with_schedule_id(
|
||||
@@ -1728,7 +1732,10 @@ mod tests {
|
||||
// Verify the messages are also preserved
|
||||
let read_messages = read_messages(&file_path)?;
|
||||
assert_eq!(read_messages.len(), 1);
|
||||
assert_eq!(read_messages[0].role, messages[0].role);
|
||||
assert_eq!(
|
||||
read_messages.first().unwrap().role,
|
||||
messages.messages()[0].role
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -1744,7 +1751,8 @@ mod tests {
|
||||
let working_dir_path = working_dir.path().to_path_buf();
|
||||
|
||||
// Create messages
|
||||
let messages = vec![Message::user().with_text("test message")];
|
||||
let messages =
|
||||
Conversation::new_unvalidated(vec![Message::user().with_text("test message")]);
|
||||
|
||||
// Get the home directory for comparison
|
||||
let home_dir = get_home_dir();
|
||||
@@ -1911,10 +1919,10 @@ mod tests {
|
||||
let dir = tempdir()?;
|
||||
let file_path = dir.path().join("test_save_session.jsonl");
|
||||
|
||||
let messages = vec![
|
||||
let messages = Conversation::new_unvalidated(vec![
|
||||
Message::user().with_text("Hello"),
|
||||
Message::assistant().with_text("Hi there"),
|
||||
];
|
||||
]);
|
||||
|
||||
let metadata = SessionMetadata::default();
|
||||
|
||||
@@ -1937,10 +1945,10 @@ mod tests {
|
||||
let dir = tempdir()?;
|
||||
let file_path = dir.path().join("test_persist_no_save.jsonl");
|
||||
|
||||
let messages = vec![
|
||||
let messages = Conversation::new_unvalidated(vec![
|
||||
Message::user().with_text("Test message"),
|
||||
Message::assistant().with_text("Test response"),
|
||||
];
|
||||
]);
|
||||
|
||||
// Test persist_messages_with_schedule_id with working_dir parameter
|
||||
persist_messages_with_schedule_id(
|
||||
|
||||
Reference in New Issue
Block a user