diff --git a/src/cortex-cli/src/feedback_cmd.rs b/src/cortex-cli/src/feedback_cmd.rs index 0b85d547..3dac5d00 100644 --- a/src/cortex-cli/src/feedback_cmd.rs +++ b/src/cortex-cli/src/feedback_cmd.rs @@ -9,7 +9,7 @@ use anyhow::{Result, bail}; use clap::Parser; use serde::Serialize; use std::io::{self, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// Feedback CLI command. #[derive(Debug, Parser)] @@ -294,19 +294,10 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> { return Ok(()); } - let mut entries = Vec::new(); + let (mut entries, warnings) = load_feedback_entries(&feedback_dir); - // Read feedback files - if let Ok(dir_entries) = std::fs::read_dir(&feedback_dir) { - for entry in dir_entries.flatten() { - let path = entry.path(); - if path.extension().is_some_and(|e| e == "json") - && let Ok(content) = std::fs::read_to_string(&path) - && let Ok(entry) = serde_json::from_str::(&content) - { - entries.push(entry); - } - } + for warning in &warnings { + eprintln!("{}", warning); } // Sort by timestamp (newest first) @@ -318,7 +309,11 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> { if args.json { println!("{}", serde_json::to_string_pretty(&entries)?); } else if entries.is_empty() { - println!("No feedback history found."); + if warnings.is_empty() { + println!("No feedback history found."); + } else { + println!("Feedback history contains unreadable entries. See warnings above."); + } } else { println!("Feedback History:"); println!("{}", "-".repeat(60)); @@ -338,6 +333,62 @@ async fn run_history(args: FeedbackHistoryArgs) -> Result<()> { Ok(()) } +fn load_feedback_entries(feedback_dir: &Path) -> (Vec, Vec) { + let mut entries = Vec::new(); + let mut warnings = Vec::new(); + + match std::fs::read_dir(feedback_dir) { + Ok(dir_entries) => { + for entry in dir_entries { + let entry = match entry { + Ok(entry) => entry, + Err(error) => { + warnings.push(format!( + "Warning: Failed to read feedback directory entry in {}: {}", + feedback_dir.display(), + error + )); + continue; + } + }; + + let path = entry.path(); + if !path.extension().is_some_and(|ext| ext == "json") { + continue; + } + + let content = match std::fs::read_to_string(&path) { + Ok(content) => content, + Err(error) => { + warnings.push(format!( + "Warning: Failed to read feedback file {}: {}", + path.display(), + error + )); + continue; + } + }; + + match serde_json::from_str::(&content) { + Ok(entry) => entries.push(entry), + Err(error) => warnings.push(format!( + "Warning: Failed to parse feedback file {}: {}", + path.display(), + error + )), + } + } + } + Err(error) => warnings.push(format!( + "Warning: Failed to read feedback directory {}: {}", + feedback_dir.display(), + error + )), + } + + (entries, warnings) +} + /// Submit feedback (save locally and optionally upload). async fn submit_feedback( category: &str, @@ -410,6 +461,7 @@ fn read_single_line() -> Result { #[cfg(test)] mod tests { use super::*; + use tempfile::tempdir; #[test] fn test_feedback_entry_serialization_with_session() { @@ -573,4 +625,56 @@ mod tests { serde_json::from_str(&pretty_json).expect("deserialization should succeed"); assert_eq!(parsed.id, entry.id); } + + #[test] + fn test_load_feedback_entries_warns_on_invalid_json() { + let temp_dir = tempdir().expect("temp dir should be created"); + let feedback_dir = temp_dir.path(); + + std::fs::write( + feedback_dir.join("bad.json"), + r#"{"id":"broken","#, + ) + .expect("invalid file should be written"); + + let (entries, warnings) = load_feedback_entries(feedback_dir); + + assert!(entries.is_empty(), "invalid JSON should not produce entries"); + assert_eq!(warnings.len(), 1, "invalid JSON should produce one warning"); + assert!( + warnings[0].contains("Failed to parse feedback file"), + "warning should mention parse failure" + ); + } + + #[test] + fn test_load_feedback_entries_keeps_valid_entries_and_collects_warnings() { + let temp_dir = tempdir().expect("temp dir should be created"); + let feedback_dir = temp_dir.path(); + + let valid_entry = FeedbackEntry { + id: "valid-entry".to_string(), + timestamp: "2024-09-01T10:00:00Z".to_string(), + category: "general".to_string(), + message: "This one is valid".to_string(), + session_id: None, + }; + + std::fs::write( + feedback_dir.join("good.json"), + serde_json::to_string(&valid_entry).expect("valid JSON should serialize"), + ) + .expect("valid file should be written"); + std::fs::write( + feedback_dir.join("bad.json"), + r#"{"id":"broken","#, + ) + .expect("invalid file should be written"); + + let (entries, warnings) = load_feedback_entries(feedback_dir); + + assert_eq!(entries.len(), 1, "valid entry should still be loaded"); + assert_eq!(entries[0].id, "valid-entry"); + assert_eq!(warnings.len(), 1, "invalid entry should still be reported"); + } }