diff --git a/.changeset/stderr-audit.md b/.changeset/stderr-audit.md new file mode 100644 index 00000000..92c9908d --- /dev/null +++ b/.changeset/stderr-audit.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Fix stdout/stderr contract: route human-readable empty-result messages and HTTP error bodies to stderr so that `gws ... | jq` pipe workflows receive valid JSON on stdout only diff --git a/src/helpers/gmail/triage.rs b/src/helpers/gmail/triage.rs index ec23bfba..8bc58804 100644 --- a/src/helpers/gmail/triage.rs +++ b/src/helpers/gmail/triage.rs @@ -76,13 +76,13 @@ pub async fn handle_triage(matches: &ArgMatches) -> Result<(), GwsError> { let messages = match list_json.get("messages").and_then(|m| m.as_array()) { Some(m) => m, None => { - println!("No messages found matching query: {query}"); + eprintln!("{}", no_messages_msg(query)); return Ok(()); } }; if messages.is_empty() { - println!("No messages found matching query: {query}"); + eprintln!("{}", no_messages_msg(query)); return Ok(()); } @@ -178,8 +178,15 @@ pub async fn handle_triage(matches: &ArgMatches) -> Result<(), GwsError> { Ok(()) } +/// Returns the human-readable "no messages" diagnostic string. +/// Extracted so the test can reference the exact same message without duplication. +fn no_messages_msg(query: &str) -> String { + format!("No messages found matching query: {query}") +} + #[cfg(test)] mod tests { + use super::no_messages_msg; use clap::{Arg, ArgAction, Command}; /// Build a clap command matching the +triage definition so we can @@ -283,4 +290,13 @@ mod tests { .unwrap_or(crate::formatter::OutputFormat::Table); assert!(matches!(fmt, crate::formatter::OutputFormat::Json)); } + + #[test] + fn empty_result_message_is_not_json() { + // Verify that no_messages_msg() produces a human-readable string that + // belongs on stderr, not stdout. If it were valid JSON it could corrupt + // pipe workflows like `gws gmail +triage | jq`. + let msg = no_messages_msg("label:inbox"); + assert!(serde_json::from_str::(&msg).is_err()); + } } diff --git a/src/helpers/modelarmor.rs b/src/helpers/modelarmor.rs index 8ac9fc1c..2fdc39fb 100644 --- a/src/helpers/modelarmor.rs +++ b/src/helpers/modelarmor.rs @@ -298,14 +298,14 @@ async fn model_armor_post(url: &str, body: &str) -> Result<(), GwsError> { let status = resp.status(); let text = resp.text().await.context("Failed to read response")?; - println!("{text}"); - if !status.is_success() { return Err(GwsError::Other(anyhow::anyhow!( - "API returned status {status}" + "API returned status {status}: {text}" ))); } + println!("{text}"); + Ok(()) } @@ -534,6 +534,19 @@ mod tests { ); } + #[test] + fn test_error_path_message_includes_status_and_body() { + // Verify that the error message produced by model_armor_post (when the + // API returns a non-2xx status) contains both the HTTP status code and + // the response body, so callers get actionable diagnostics. + // Uses reqwest::StatusCode to mirror the type used in the actual function. + let status = reqwest::StatusCode::FORBIDDEN; + let body = r#"{"error":{"message":"permission denied"}}"#; + let msg = format!("API returned status {status}: {body}"); + assert!(msg.contains("403 Forbidden")); + assert!(msg.contains("permission denied")); + } + #[test] fn test_build_sanitize_request_data() { let template = "projects/p/locations/us-central1/templates/t";