From 982fc23f3b08d4272a9ad4e88fc8b34d905fba40 Mon Sep 17 00:00:00 2001 From: Alex Mikhalev Date: Mon, 13 Apr 2026 13:20:39 +0200 Subject: [PATCH] fix(ci): stabilize release build test suites Make terraphim_agent role-selection and persistence tests derive valid roles from the live config, and make terraphim_mcp_server integration tests pass zlob to their nested cargo build under CI so the main release build stops failing on stale role assumptions and fff build-script panics. --- .../tests/persistence_tests.rs | 124 +++++++++++++----- .../tests/selected_role_tests.rs | 49 +++++-- .../tests/integration_test.rs | 17 ++- 3 files changed, 145 insertions(+), 45 deletions(-) diff --git a/crates/terraphim_agent/tests/persistence_tests.rs b/crates/terraphim_agent/tests/persistence_tests.rs index aa9bd95bb..690f1804d 100644 --- a/crates/terraphim_agent/tests/persistence_tests.rs +++ b/crates/terraphim_agent/tests/persistence_tests.rs @@ -45,6 +45,57 @@ fn parse_config_from_output(output: &str) -> Result { Ok(serde_json::from_str(&json_str)?) } +/// Parse role names from `terraphim-agent roles list` output. +fn list_available_roles() -> Result> { + let (stdout, stderr, code) = run_tui_command(&["roles", "list"])?; + anyhow::ensure!(code == 0, "roles list should succeed, stderr: {}", stderr); + + let roles = extract_clean_output(&stdout) + .lines() + .filter_map(|line| { + let trimmed = line.trim_start(); + if trimmed.is_empty() { + return None; + } + + let rest = trimmed + .strip_prefix('*') + .or_else(|| trimmed.strip_prefix('-')) + .map(str::trim_start) + .unwrap_or(trimmed); + + let name = rest + .split_once(" (") + .map(|(name, _)| name) + .unwrap_or(rest) + .trim(); + + if name.is_empty() { + None + } else { + Some(name.to_string()) + } + }) + .collect::>(); + + anyhow::ensure!(!roles.is_empty(), "roles list returned no roles"); + Ok(roles) +} + +fn pick_existing_role(roles: &[String], preferred: &[&str]) -> String { + preferred + .iter() + .find_map(|candidate| roles.iter().find(|role| role.as_str() == *candidate)) + .cloned() + .unwrap_or_else(|| roles[0].clone()) +} + +fn sample_roles(roles: &[String], count: usize) -> Vec { + (0..count) + .map(|idx| roles[idx % roles.len()].clone()) + .collect() +} + /// Clean up test persistence files fn cleanup_test_persistence() -> Result<()> { // Clean up test persistence directories @@ -103,12 +154,13 @@ async fn test_persistence_setup_and_cleanup() -> Result<()> { #[serial] async fn test_config_persistence_across_runs() -> Result<()> { cleanup_test_persistence()?; + let roles = list_available_roles()?; // First run: Set a configuration value // selected_role must be an existing role name - let test_role = "Rust Engineer"; + let test_role = pick_existing_role(&roles, &["Rust Engineer", "Terraphim Engineer", "Default"]); let (stdout1, stderr1, code1) = - run_tui_command(&["config", "set", "selected_role", test_role])?; + run_tui_command(&["config", "set", "selected_role", &test_role])?; assert_eq!( code1, 0, @@ -147,10 +199,11 @@ async fn test_config_persistence_across_runs() -> Result<()> { #[serial] async fn test_role_switching_persistence() -> Result<()> { cleanup_test_persistence()?; + let available_roles = list_available_roles()?; // Test switching between different roles and verifying persistence // selected_role must be an existing role name - let roles_to_test = ["Default", "Rust Engineer", "Terraphim Engineer", "Default"]; + let roles_to_test = sample_roles(&available_roles, 4); for (i, role) in roles_to_test.iter().enumerate() { println!("Testing role switch #{}: '{}'", i + 1, role); @@ -202,11 +255,7 @@ async fn test_role_switching_persistence() -> Result<()> { let final_role = final_config["selected_role"].as_str().unwrap(); // NOTE: persistence across runs is not required; just ensure we end up with a valid role - assert!( - final_role == "Default" - || final_role == "Rust Engineer" - || final_role == "Terraphim Engineer" - ); + assert!(roles_to_test.iter().any(|role| role == final_role)); println!( "✓ Role switching completed; final selected_role: '{}'", final_role @@ -219,19 +268,19 @@ async fn test_role_switching_persistence() -> Result<()> { #[serial] async fn test_persistence_backend_functionality() -> Result<()> { cleanup_test_persistence()?; + let roles = list_available_roles()?; // Test that different persistence backends work // Run multiple operations to exercise the persistence layer // Set multiple config values - let config_changes = vec![ - ("selected_role", "Default"), - ("selected_role", "Rust Engineer"), - ("selected_role", "Terraphim Engineer"), - ]; + let config_changes = sample_roles(&roles, 3) + .into_iter() + .map(|role| ("selected_role", role)) + .collect::>(); for (key, value) in config_changes { - let (_stdout, stderr, code) = run_tui_command(&["config", "set", key, value])?; + let (_stdout, stderr, code) = run_tui_command(&["config", "set", key, &value])?; assert_eq!( code, 0, @@ -265,22 +314,17 @@ async fn test_persistence_backend_functionality() -> Result<()> { #[serial] async fn test_concurrent_persistence_operations() -> Result<()> { cleanup_test_persistence()?; + let available_roles = list_available_roles()?; // Test that concurrent TUI operations don't corrupt persistence // Run multiple TUI commands simultaneously // Use existing roles for concurrent operations (arbitrary role names are rejected) - let roles = [ - "Default", - "Rust Engineer", - "Terraphim Engineer", - "Default", - "Rust Engineer", - ]; + let roles = sample_roles(&available_roles, 5); let handles: Vec<_> = (0..5) .map(|i| { - let role = roles[i].to_string(); + let role = roles[i].clone(); tokio::spawn(async move { let result = run_tui_command(&["config", "set", "selected_role", &role]); (i, role, result) @@ -327,10 +371,8 @@ async fn test_concurrent_persistence_operations() -> Result<()> { // Should have one of the concurrent roles assert!( - final_role == "Default" - || final_role == "Rust Engineer" - || final_role == "Terraphim Engineer", - "Final role should be one of the known roles: '{}'", + roles.iter().any(|role| role == final_role), + "Final role should be one of the selected roles: '{}'", final_role ); @@ -346,9 +388,14 @@ async fn test_concurrent_persistence_operations() -> Result<()> { #[serial] async fn test_persistence_recovery_after_corruption() -> Result<()> { cleanup_test_persistence()?; + let roles = list_available_roles()?; + let initial_role = + pick_existing_role(&roles, &["Default", "Terraphim Engineer", "Rust Engineer"]); + let recovery_role = + pick_existing_role(&roles, &["Rust Engineer", "Terraphim Engineer", "Default"]); // First, set up normal persistence - let (_, stderr1, code1) = run_tui_command(&["config", "set", "selected_role", "Default"])?; + let (_, stderr1, code1) = run_tui_command(&["config", "set", "selected_role", &initial_role])?; assert_eq!( code1, 0, "Initial setup should succeed, stderr: {}", @@ -388,8 +435,7 @@ async fn test_persistence_recovery_after_corruption() -> Result<()> { ); // Should be able to set new values - let (_, stderr2, code2) = - run_tui_command(&["config", "set", "selected_role", "Rust Engineer"])?; + let (_, stderr2, code2) = run_tui_command(&["config", "set", "selected_role", &recovery_role])?; assert_eq!( code2, 0, "Should be able to set config after recovery, stderr: {}", @@ -405,16 +451,26 @@ async fn test_persistence_recovery_after_corruption() -> Result<()> { #[serial] async fn test_persistence_with_special_characters() -> Result<()> { cleanup_test_persistence()?; + let roles = list_available_roles()?; // selected_role must be an existing role name; arbitrary strings are rejected. // This test only verifies that roles containing spaces (existing in the config) can be set. - let special_roles = vec!["Rust Engineer", "Terraphim Engineer"]; + let special_roles = roles + .iter() + .filter(|role| role.contains(' ')) + .cloned() + .collect::>(); + + anyhow::ensure!( + !special_roles.is_empty(), + "expected at least one role containing spaces in roles list" + ); for role in special_roles { println!("Testing persistence with special role: '{}'", role); let (_set_stdout, set_stderr, set_code) = - run_tui_command(&["config", "set", "selected_role", role])?; + run_tui_command(&["config", "set", "selected_role", &role])?; assert_eq!( set_code, 0, @@ -481,11 +537,15 @@ async fn test_persistence_directory_permissions() -> Result<()> { #[serial] async fn test_persistence_backend_selection() -> Result<()> { cleanup_test_persistence()?; + let roles = list_available_roles()?; + let selected_role = + pick_existing_role(&roles, &["Default", "Terraphim Engineer", "Rust Engineer"]); // Test that the TUI uses the expected persistence backends // Based on settings, it should use multiple backends for redundancy - let (_stdout, stderr, code) = run_tui_command(&["config", "set", "selected_role", "Default"])?; + let (_stdout, stderr, code) = + run_tui_command(&["config", "set", "selected_role", &selected_role])?; assert_eq!(code, 0, "Config set should succeed, stderr: {}", stderr); // Check that expected backends are being used (from log output) diff --git a/crates/terraphim_agent/tests/selected_role_tests.rs b/crates/terraphim_agent/tests/selected_role_tests.rs index 94e66face..0aa73bf24 100644 --- a/crates/terraphim_agent/tests/selected_role_tests.rs +++ b/crates/terraphim_agent/tests/selected_role_tests.rs @@ -139,10 +139,22 @@ async fn test_default_selected_role_is_used() -> Result<()> { #[serial] async fn test_role_override_in_commands() -> Result<()> { // Test that --role flag overrides selected_role in config + let available_roles = fetch_available_roles()?; + ensure!( + !available_roles.is_empty(), + "Expected at least one role for override test" + ); + let override_role = available_roles[0].clone(); // Search with role override - let (search_stdout, search_stderr, search_code) = - run_command_and_parse(&["search", "test query", "--role", "Default", "--limit", "3"])?; + let (search_stdout, search_stderr, search_code) = run_command_and_parse(&[ + "search", + "test query", + "--role", + override_role.as_str(), + "--limit", + "3", + ])?; // Should succeed or fail gracefully (depending on whether role exists) assert!( @@ -158,7 +170,7 @@ async fn test_role_override_in_commands() -> Result<()> { // Graph with role override let (graph_stdout, graph_stderr, graph_code) = - run_command_and_parse(&["graph", "--role", "Default", "--top-k", "5"])?; + run_command_and_parse(&["graph", "--role", override_role.as_str(), "--top-k", "5"])?; assert_eq!( graph_code, 0, @@ -173,7 +185,7 @@ async fn test_role_override_in_commands() -> Result<()> { // Chat with role override let (chat_stdout, chat_stderr, chat_code) = - run_command_and_parse(&["chat", "test message", "--role", "Default"])?; + run_command_and_parse(&["chat", "test message", "--role", override_role.as_str()])?; // Chat may succeed (with proxy/ollama) or fail gracefully // Accept exit code 0 or 1 @@ -188,7 +200,7 @@ async fn test_role_override_in_commands() -> Result<()> { // Accept: non-empty output, role name in output, error message, or proxy error let success = !chat_stdout.trim().is_empty() - || combined_output.contains("Default") + || combined_output.contains(&override_role) || combined_output.contains("No LLM") || combined_output.contains("Failed to connect") || combined_output.contains("terraphim-llm-proxy"); @@ -416,13 +428,25 @@ async fn test_role_inheritance_in_search() -> Result<()> { run_command_and_parse(&["config", "set", "selected_role", test_role.as_str()])?; assert_eq!(set_code, 0, "Should set test role"); + let override_role = available_roles + .iter() + .find(|role| role.as_str() != test_role) + .cloned() + .unwrap_or_else(|| test_role.clone()); + // Search without specifying role (should use selected_role) let (search1_stdout, search1_stderr, search1_code) = run_command_and_parse(&["search", "test query", "--limit", "2"])?; // Search with explicit role override - let (search2_stdout, search2_stderr, search2_code) = - run_command_and_parse(&["search", "test query", "--role", "Default", "--limit", "2"])?; + let (search2_stdout, search2_stderr, search2_code) = run_command_and_parse(&[ + "search", + "test query", + "--role", + override_role.as_str(), + "--limit", + "2", + ])?; // Both should handle the role appropriately (succeed or fail gracefully) assert!( @@ -442,7 +466,8 @@ async fn test_role_inheritance_in_search() -> Result<()> { extract_clean_output(&search1_stdout) ); println!( - "Search with role override 'Default': {}", + "Search with role override '{}': {}", + override_role, extract_clean_output(&search2_stdout) ); @@ -465,13 +490,19 @@ async fn test_extract_command_role_behavior() -> Result<()> { run_command_and_parse(&["config", "set", "selected_role", test_role.as_str()])?; assert_eq!(set_code, 0, "Should set test role"); + let override_role = available_roles + .iter() + .find(|role| role.as_str() != test_role) + .cloned() + .unwrap_or_else(|| test_role.clone()); + // Extract without role (should use selected_role) let (extract1_stdout, extract1_stderr, extract1_code) = run_command_and_parse(&["extract", test_text])?; // Extract with role override let (extract2_stdout, extract2_stderr, extract2_code) = - run_command_and_parse(&["extract", test_text, "--role", "Default"])?; + run_command_and_parse(&["extract", test_text, "--role", override_role.as_str()])?; // Extract with exclude term flag let (extract3_stdout, extract3_stderr, extract3_code) = diff --git a/crates/terraphim_mcp_server/tests/integration_test.rs b/crates/terraphim_mcp_server/tests/integration_test.rs index 4f5c4944f..bd66cb520 100644 --- a/crates/terraphim_mcp_server/tests/integration_test.rs +++ b/crates/terraphim_mcp_server/tests/integration_test.rs @@ -11,12 +11,21 @@ use tokio::process::Command; async fn setup_server_command() -> Result { // Build the server first to ensure the binary is up-to-date - let build_status = Command::new("cargo") + let mut build = Command::new("cargo"); + build .arg("build") .arg("--package") - .arg("terraphim_mcp_server") - .status() - .await?; + .arg("terraphim_mcp_server"); + + // CI sets CI=true, and terraphim_mcp_server depends on fff-search whose + // build script requires the zlob feature under CI. The top-level main + // workflow already runs the workspace tests with zlob enabled, so mirror + // that feature contract for this nested build as well. + if std::env::var_os("CI").is_some() { + build.arg("--features").arg("zlob"); + } + + let build_status = build.status().await?; if !build_status.success() { return Err(anyhow::anyhow!("Failed to build terraphim_mcp_server")); }