From 2db074cbfe4d470e4fd99c5204781b54db8b712a Mon Sep 17 00:00:00 2001 From: AlexMikhalev Date: Fri, 7 Aug 2026 12:51:07 +0100 Subject: [PATCH] fix(agent): honor global JSON output format - apply global --format json/json-compact to guard/validate/suggest - preserve legacy subcommand --json compact one-line output - cover offline and server-mode fallback format contracts --- crates/terraphim_agent/src/main.rs | 52 ++-- .../tests/agent_format_contract_tests.rs | 232 ++++++++++++++++++ 2 files changed, 268 insertions(+), 16 deletions(-) create mode 100644 crates/terraphim_agent/tests/agent_format_contract_tests.rs diff --git a/crates/terraphim_agent/src/main.rs b/crates/terraphim_agent/src/main.rs index 291d18a42..85e26210b 100644 --- a/crates/terraphim_agent/src/main.rs +++ b/crates/terraphim_agent/src/main.rs @@ -587,6 +587,24 @@ fn resolve_output_config(robot: bool, format: OutputFormat) -> CommandOutputConf CommandOutputConfig { mode, robot } } +fn effective_json_output_mode( + subcommand_json: bool, + output: CommandOutputConfig, +) -> Option { + if !subcommand_json && !output.is_machine_readable() { + return None; + } + + Some( + if subcommand_json && matches!(output.mode, CommandOutputMode::Human) { + // Historical subcommand `--json` output used compact, single-line JSON. + CommandOutputMode::JsonCompact + } else { + output.mode + }, + ) +} + #[cfg(feature = "repl-sessions")] mod session_output { use serde::Serialize; @@ -1988,8 +2006,8 @@ async fn run_offline_command( }; let result = guard.check(&input_command); - if *json { - println!("{}", serde_json::to_string(&result)?); + if let Some(json_mode) = effective_json_output_mode(*json, output) { + print_json_output(&result, json_mode)?; } else if result.decision == guard_patterns::GuardDecision::Block { if let Some(reason) = &result.reason { eprintln!("BLOCKED: {}", reason); @@ -2570,12 +2588,13 @@ async fn run_offline_command( }; let role_name = service.resolve_role(role.as_deref()).await?; + let json_mode = effective_json_output_mode(json, output); if connectivity { let result = service.check_connectivity(&role_name, &input_text).await?; - if json { - println!("{}", serde_json::to_string(&result)?); + if let Some(json_mode) = json_mode { + print_json_output(&result, json_mode)?; } else { println!("Connectivity Check for role '{}':", role_name); println!(" Connected: {}", result.connected); @@ -2588,8 +2607,8 @@ async fn run_offline_command( .validate_checklist(&role_name, &checklist_name, &input_text) .await?; - if json { - println!("{}", serde_json::to_string(&result)?); + if let Some(json_mode) = json_mode { + print_json_output(&result, json_mode)?; } else { println!( "Checklist '{}' Validation for role '{}':", @@ -2614,13 +2633,13 @@ async fn run_offline_command( // Default validation: find matches let matches = service.find_matches(&role_name, &input_text).await?; - if json { + if let Some(json_mode) = json_mode { let output = serde_json::json!({ "role": role_name.to_string(), "matched_count": matches.len(), "matches": matches.iter().map(|m| m.term.clone()).collect::>() }); - println!("{}", serde_json::to_string(&output)?); + print_json_output(&output, json_mode)?; } else { println!("Validation for role '{}':", role_name); println!(" Found {} matched term(s)", matches.len()); @@ -2651,13 +2670,14 @@ async fn run_offline_command( }; let role_name = service.resolve_role(role.as_deref()).await?; + let json_mode = effective_json_output_mode(json, output); let suggestions = service .fuzzy_suggest(&role_name, &input_query, threshold, Some(limit)) .await?; - if json { - println!("{}", serde_json::to_string(&suggestions)?); + if let Some(json_mode) = json_mode { + print_json_output(&suggestions, json_mode)?; } else if suggestions.is_empty() { println!( "No suggestions found for '{}' with threshold {}", @@ -4669,22 +4689,22 @@ async fn run_server_command( } } Command::Validate { json, .. } => { - if json { + if let Some(json_mode) = effective_json_output_mode(json, output) { let err = serde_json::json!({ "error": "Validate command is only available in offline mode" }); - println!("{}", serde_json::to_string(&err)?); + print_json_output(&err, json_mode)?; } else { eprintln!("Validate command is only available in offline mode"); } std::process::exit(1); } Command::Suggest { json, .. } => { - if json { + if let Some(json_mode) = effective_json_output_mode(json, output) { let err = serde_json::json!({ "error": "Suggest command is only available in offline mode" }); - println!("{}", serde_json::to_string(&err)?); + print_json_output(&err, json_mode)?; } else { eprintln!("Suggest command is only available in offline mode"); } @@ -4748,8 +4768,8 @@ async fn run_server_command( }; let result = guard.check(&input_command); - if json { - println!("{}", serde_json::to_string(&result)?); + if let Some(json_mode) = effective_json_output_mode(json, output) { + print_json_output(&result, json_mode)?; } else if result.decision == guard_patterns::GuardDecision::Block { if let Some(reason) = &result.reason { eprintln!("BLOCKED: {}", reason); diff --git a/crates/terraphim_agent/tests/agent_format_contract_tests.rs b/crates/terraphim_agent/tests/agent_format_contract_tests.rs new file mode 100644 index 000000000..1fba6ef40 --- /dev/null +++ b/crates/terraphim_agent/tests/agent_format_contract_tests.rs @@ -0,0 +1,232 @@ +use anyhow::{Context, Result}; +use serde_json::Value; +use std::process::Command; + +mod support; +use support::cli_test_env::apply_hermetic_env; + +fn run_agent(args: &[&str]) -> Result<(String, String, i32)> { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_terraphim-agent")); + cmd.args(args); + apply_hermetic_env(&mut cmd)?; + + let output = cmd.output().context("run terraphim-agent")?; + + Ok(( + String::from_utf8_lossy(&output.stdout).to_string(), + String::from_utf8_lossy(&output.stderr).to_string(), + output.status.code().unwrap_or(-1), + )) +} + +#[test] +fn global_json_compact_guard_emits_single_line_json() -> Result<()> { + let (stdout, stderr, code) = + run_agent(&["--format", "json-compact", "guard", "git reset --hard HEAD"])?; + + assert_eq!( + code, 0, + "machine-readable guard should preserve --json compatibility; stderr={stderr}" + ); + assert!( + !stderr.contains("BLOCKED:"), + "human BLOCKED text must not be emitted for global JSON format: {stderr}" + ); + + let trimmed = stdout.trim(); + assert!(!trimmed.is_empty(), "stdout should contain JSON"); + assert_eq!( + trimmed.lines().count(), + 1, + "json-compact should be single-line" + ); + let json: Value = serde_json::from_str(trimmed)?; + assert_eq!(json["decision"], "block"); + assert_eq!(json["command"], "git reset --hard HEAD"); + Ok(()) +} + +#[test] +fn global_json_validate_emits_parseable_json() -> Result<()> { + let (stdout, stderr, code) = run_agent(&["--format", "json", "validate", "terraphim"])?; + + assert_eq!(code, 0, "validate should succeed; stderr={stderr}"); + let json: Value = serde_json::from_str(stdout.trim())?; + assert!( + json.get("matched_count").is_some() || json.get("error").is_some(), + "unexpected validate JSON: {json}" + ); + Ok(()) +} + +#[test] +fn global_json_guard_emits_pretty_json() -> Result<()> { + let (stdout, stderr, code) = + run_agent(&["--format", "json", "guard", "git reset --hard HEAD"])?; + + assert_eq!( + code, 0, + "global --format json guard should succeed; stderr={stderr}" + ); + assert!( + !stderr.contains("BLOCKED:"), + "human BLOCKED text must not be emitted for global JSON format: {stderr}" + ); + let json = assert_pretty_json(&stdout)?; + assert_eq!(json["decision"], "block"); + assert_eq!(json["command"], "git reset --hard HEAD"); + Ok(()) +} + +#[test] +fn global_json_compact_suggest_emits_parseable_json() -> Result<()> { + let (stdout, stderr, code) = run_agent(&[ + "--format", + "json-compact", + "suggest", + "terraphim", + "--limit", + "3", + ])?; + + assert_eq!(code, 0, "suggest should succeed; stderr={stderr}"); + assert_single_line_json(&stdout)?; + Ok(()) +} + +#[test] +fn legacy_guard_json_stays_single_line_compact() -> Result<()> { + let (stdout, stderr, code) = run_agent(&["guard", "--json", "git reset --hard HEAD"])?; + + assert_eq!( + code, 0, + "legacy guard --json compatibility; stderr={stderr}" + ); + assert!( + !stderr.contains("BLOCKED:"), + "legacy JSON guard should not emit human BLOCKED text: {stderr}" + ); + let json = assert_single_line_json(&stdout)?; + assert_eq!(json["decision"], "block"); + Ok(()) +} + +#[test] +fn legacy_validate_json_stays_single_line_compact() -> Result<()> { + let (stdout, stderr, code) = run_agent(&["validate", "--json", "terraphim"])?; + + assert_eq!( + code, 0, + "legacy validate --json compatibility; stderr={stderr}" + ); + let json = assert_single_line_json(&stdout)?; + assert!( + json.get("matched_count").is_some() || json.get("error").is_some(), + "unexpected validate JSON: {json}" + ); + Ok(()) +} + +#[test] +fn legacy_suggest_json_stays_single_line_compact() -> Result<()> { + let (stdout, stderr, code) = run_agent(&["suggest", "--json", "terraphim", "--limit", "3"])?; + + assert_eq!( + code, 0, + "legacy suggest --json compatibility; stderr={stderr}" + ); + assert_single_line_json(&stdout)?; + Ok(()) +} + +#[test] +fn server_mode_global_json_compact_guard_emits_single_line_json() -> Result<()> { + let (stdout, stderr, code) = run_agent(&[ + "--server", + "--format", + "json-compact", + "guard", + "git reset --hard HEAD", + ])?; + + assert_eq!( + code, 0, + "machine-readable server-mode guard should preserve --json compatibility; stderr={stderr}" + ); + assert!( + !stderr.contains("BLOCKED:"), + "human BLOCKED text must not be emitted for server-mode global JSON format: {stderr}" + ); + let json = assert_single_line_json(&stdout)?; + assert_eq!(json["decision"], "block"); + Ok(()) +} + +#[test] +fn server_mode_global_json_validate_error_is_parseable_json() -> Result<()> { + let (stdout, stderr, code) = run_agent(&[ + "--server", + "--format", + "json-compact", + "validate", + "terraphim", + ])?; + + assert_eq!(code, 1, "server-mode validate should remain unavailable"); + assert!( + !stderr.contains("Validate command is only available in offline mode"), + "machine-readable unavailable error must be stdout JSON, not human stderr: {stderr}" + ); + let json = assert_single_line_json(&stdout)?; + assert_eq!( + json["error"], + "Validate command is only available in offline mode" + ); + Ok(()) +} + +#[test] +fn server_mode_global_json_compact_suggest_error_is_parseable_json() -> Result<()> { + let (stdout, stderr, code) = run_agent(&[ + "--server", + "--format", + "json-compact", + "suggest", + "terraphim", + "--limit", + "3", + ])?; + + assert_eq!(code, 1, "server-mode suggest should remain unavailable"); + assert!( + !stderr.contains("Suggest command is only available in offline mode"), + "machine-readable unavailable error must be stdout JSON, not human stderr: {stderr}" + ); + let json = assert_single_line_json(&stdout)?; + assert_eq!( + json["error"], + "Suggest command is only available in offline mode" + ); + Ok(()) +} + +fn assert_single_line_json(stdout: &str) -> Result { + let trimmed = stdout.trim(); + assert!(!trimmed.is_empty(), "stdout should contain JSON"); + assert_eq!( + trimmed.lines().count(), + 1, + "json-compact should be single-line" + ); + Ok(serde_json::from_str(trimmed)?) +} + +fn assert_pretty_json(stdout: &str) -> Result { + let trimmed = stdout.trim(); + assert!(!trimmed.is_empty(), "stdout should contain JSON"); + assert!( + trimmed.lines().count() > 1, + "--format json should be pretty multi-line JSON; stdout={trimmed}" + ); + Ok(serde_json::from_str(trimmed)?) +}