diff --git a/src/api.rs b/src/api.rs index 9449f5b..4eae4b1 100644 --- a/src/api.rs +++ b/src/api.rs @@ -10,7 +10,7 @@ use tokio::sync::RwLock; use crate::cache::{Cache, CacheOptions, CacheType}; use crate::config; -use crate::error::CliError; +use crate::error::{CliError, ErrorKind}; use crate::pagination::{paginate_nodes, PaginationOptions}; use crate::retry::{with_retry, RetryConfig}; use crate::text::is_uuid; @@ -137,6 +137,25 @@ fn http_error(status: StatusCode, headers: &HeaderMap, context: &str) -> CliErro err.with_details(details) } +/// Refine a transport-level HTTP error using its GraphQL payload. +/// +/// Linear can return application errors (notably RATELIMITED) as a non-2xx +/// status carrying a GraphQL `errors` array, so the transport status alone +/// under-classifies them. When the payload resolves to a more specific kind +/// than the status, prefer it so the exit code stays precise (e.g. 4 for a +/// rate limit behind HTTP 400). The status-derived message is kept. +fn refine_http_error(http_err: CliError, body: &Value) -> CliError { + let Some(errors) = body.get("errors") else { + return http_err; + }; + let payload = CliError::from_graphql_errors(errors); + if payload.kind == ErrorKind::General { + return http_err; + } + CliError::new(payload.kind, http_err.message.clone()) + .with_retry_after(http_err.retry_after.or(payload.retry_after)) +} + pub fn parse_linear_upload_url(raw: &str) -> Result { let url = Url::parse(raw).context("Failed to parse upload URL")?; @@ -685,7 +704,7 @@ impl LinearClient { } else { json!({ "body": body }) }; - let mut err = http_error(status, &headers, "resource"); + let mut err = refine_http_error(http_error(status, &headers, "resource"), &details); if !body.is_empty() { err = err.with_details(details); } @@ -695,9 +714,7 @@ impl LinearClient { let result: Value = response.json().await?; if let Some(errors) = result.get("errors") { - return Err(CliError::general("GraphQL error") - .with_details(errors.clone()) - .into()); + return Err(CliError::from_graphql_errors(errors).into()); } Ok(result) @@ -871,6 +888,41 @@ fn default_retry_config() -> RetryConfig { mod tests { use super::*; + #[test] + fn refine_http_error_prefers_ratelimited_payload_behind_http_400() { + // Linear returns GraphQL rate limits as HTTP 400 + extensions.code + // RATELIMITED; the transport status alone would yield General(1). + let http_err = CliError::general("HTTP 400 Bad Request"); + let body = json!({ + "errors": [{ + "message": "Rate limit exceeded", + "extensions": { "code": "RATELIMITED", "retryAfter": 12 } + }] + }); + let refined = refine_http_error(http_err, &body); + assert_eq!(refined.kind, ErrorKind::RateLimited); + assert_eq!(refined.code(), 4); + assert_eq!(refined.retry_after, Some(12)); + } + + #[test] + fn refine_http_error_keeps_status_kind_when_payload_is_generic() { + // A 401 whose payload doesn't classify must stay Auth(3), not regress to General. + let http_err = CliError::auth("Authentication failed"); + let body = json!({ "errors": [{ "message": "Unauthorized" }] }); + let refined = refine_http_error(http_err, &body); + assert_eq!(refined.kind, ErrorKind::Auth); + assert_eq!(refined.code(), 3); + } + + #[test] + fn refine_http_error_passes_through_without_errors_array() { + let http_err = CliError::not_found("resource not found"); + let body = json!({ "body": "502" }); + let refined = refine_http_error(http_err, &body); + assert_eq!(refined.kind, ErrorKind::NotFound); + } + #[test] fn test_auth_state_api_key_header() { let state = AuthState::ApiKey("lin_api_key123".to_string()); diff --git a/src/commands/bulk.rs b/src/commands/bulk.rs index ab1fa47..0a7f402 100644 --- a/src/commands/bulk.rs +++ b/src/commands/bulk.rs @@ -10,6 +10,7 @@ use tokio::sync::Mutex; use crate::api::{resolve_label_id, resolve_state_id, resolve_user_id, LinearClient}; use crate::display_options; +use crate::error::{self, CliError}; use crate::output::{print_json_owned, OutputOptions}; use crate::text::truncate; @@ -148,8 +149,7 @@ async fn bulk_update_state(state: &str, issues: Vec, output: &OutputOpti .collect() .await; print_summary(&results, "state updated", output); - - Ok(()) + bulk_exit_status(&results) } async fn bulk_assign(user: &str, issues: Vec, output: &OutputOptions) -> Result<()> { @@ -168,21 +168,12 @@ async fn bulk_assign(user: &str, issues: Vec, output: &OutputOptions) -> let client = LinearClient::new()?; - // Resolve the user ID once upfront - let user_id = match resolve_user_id(&client, user, &output.cache).await { - Ok(id) => id, - Err(e) => { - if output.is_json() || output.has_template() { - print_json_owned( - json!({ "error": format!("Failed to resolve user '{}': {}", user, e), "results": [] }), - output, - )?; - } else { - println!("{} Failed to resolve user '{}': {}", "x".red(), user, e); - } - return Ok(()); - } - }; + // Resolution failure is a hard error: return Err, preserving the kind. + let user_id = resolve_user_id(&client, user, &output.cache) + .await + .map_err(|e| { + error::rewrap_with_message(&e, format!("Failed to resolve user '{user}': {e}")) + })?; let results: Vec<_> = stream::iter(issues.iter()) .map(|issue_id| { @@ -195,8 +186,7 @@ async fn bulk_assign(user: &str, issues: Vec, output: &OutputOptions) -> .collect() .await; print_summary(&results, "assigned", output); - - Ok(()) + bulk_exit_status(&results) } async fn bulk_label(label: &str, issues: Vec, output: &OutputOptions) -> Result<()> { @@ -215,21 +205,12 @@ async fn bulk_label(label: &str, issues: Vec, output: &OutputOptions) -> let client = LinearClient::new()?; - // Resolve the label ID once upfront - let label_id = match resolve_label_id(&client, label, &output.cache).await { - Ok(id) => id, - Err(e) => { - if output.is_json() || output.has_template() { - print_json_owned( - json!({ "error": format!("Failed to resolve label '{}': {}", label, e), "results": [] }), - output, - )?; - } else { - println!("{} Failed to resolve label '{}': {}", "x".red(), label, e); - } - return Ok(()); - } - }; + // Resolution failure is a hard error: return Err, preserving the kind. + let label_id = resolve_label_id(&client, label, &output.cache) + .await + .map_err(|e| { + error::rewrap_with_message(&e, format!("Failed to resolve label '{label}': {e}")) + })?; let results: Vec<_> = stream::iter(issues.iter()) .map(|issue_id| { @@ -242,8 +223,7 @@ async fn bulk_label(label: &str, issues: Vec, output: &OutputOptions) -> .collect() .await; print_summary(&results, "labeled", output); - - Ok(()) + bulk_exit_status(&results) } async fn bulk_unassign(issues: Vec, output: &OutputOptions) -> Result<()> { @@ -267,8 +247,7 @@ async fn bulk_unassign(issues: Vec, output: &OutputOptions) -> Result<() .collect() .await; print_summary(&results, "unassigned", output); - - Ok(()) + bulk_exit_status(&results) } fn missing_bulk_issues(example: &str) -> Result<()> { @@ -620,3 +599,52 @@ fn print_summary(results: &[BulkResult], action: &str, output: &OutputOptions) { } ); } + +/// `Err` if any item failed, so partial failures still exit non-zero. +fn bulk_exit_status(results: &[BulkResult]) -> Result<()> { + let failed = results.iter().filter(|r| !r.success).count(); + if failed > 0 { + return Err(CliError::general(format!( + "{} of {} operations failed", + failed, + results.len() + )) + .into()); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn result(success: bool) -> BulkResult { + BulkResult { + issue_id: "LIN-1".to_string(), + success, + identifier: None, + error: if success { + None + } else { + Some("boom".to_string()) + }, + } + } + + #[test] + fn bulk_exit_status_ok_when_all_succeed_or_empty() { + assert!(bulk_exit_status(&[]).is_ok()); + assert!(bulk_exit_status(&[result(true), result(true)]).is_ok()); + } + + #[test] + fn bulk_exit_status_errs_on_any_failure() { + let err = bulk_exit_status(&[result(true), result(false)]).unwrap_err(); + assert_eq!(err.downcast_ref::().expect("CliError").code(), 1); + } + + #[test] + fn bulk_exit_status_errs_when_all_fail() { + assert!(bulk_exit_status(&[result(false), result(false)]).is_err()); + } +} diff --git a/src/commands/comments.rs b/src/commands/comments.rs index 8e64519..504f51a 100644 --- a/src/commands/comments.rs +++ b/src/commands/comments.rs @@ -7,6 +7,7 @@ use tabled::{Table, Tabled}; use crate::api::LinearClient; use crate::display_options; +use crate::error::{CliError, ErrorKind}; use crate::input::read_ids_from_stdin; use crate::output::{ ensure_non_empty, filter_values, print_json, print_json_owned, sort_values, OutputOptions, @@ -109,7 +110,10 @@ async fn list_comments(issue_ids: &[String], output: &OutputOptions) -> Result<( Err(e) => Err((id, e)), } } - Ok(_) => Err((id, anyhow::anyhow!("Issue not found"))), + Ok(_) => Err(( + id.clone(), + CliError::not_found(format!("Issue not found: {id}")).into(), + )), Err(e) => Err((id, e)), } } @@ -119,12 +123,28 @@ async fn list_comments(issue_ids: &[String], output: &OutputOptions) -> Result<( .await; let mut issues = Vec::new(); + let mut missing: Vec = Vec::new(); + // Real fetch errors keep their kind; missing issues map to NotFound. + let mut first_error: Option = None; for result in fetched { match result { Ok((_id, issue)) => issues.push(issue), - Err((id, _e)) => { + Err((id, e)) => { + let is_missing = e + .downcast_ref::() + .map(|c| c.kind == ErrorKind::NotFound) + .unwrap_or(false); if !output.is_json() && !output.has_template() { - eprintln!("{} Issue not found: {}", "!".yellow(), id); + if is_missing { + eprintln!("{} Issue not found: {}", "!".yellow(), id); + } else { + eprintln!("{} Error fetching {}: {}", "!".red(), id, e); + } + } + if is_missing { + missing.push(id); + } else if first_error.is_none() { + first_error = Some(e); } } } @@ -137,7 +157,7 @@ async fn list_comments(issue_ids: &[String], output: &OutputOptions) -> Result<( } else { print_json_owned(serde_json::json!(issues), output)?; } - return Ok(()); + return comments_status(first_error, &missing); } for (idx, issue) in issues.iter().enumerate() { @@ -202,7 +222,20 @@ async fn list_comments(issue_ids: &[String], output: &OutputOptions) -> Result<( println!("\n{} comments", comments.len()); } - Ok(()) + comments_status(first_error, &missing) +} + +/// Aggregate exit status: a real fetch error (passed through unchanged) wins so +/// its kind survives; otherwise missing issues map to NotFound. +fn comments_status(first_error: Option, missing: &[String]) -> Result<()> { + if let Some(e) = first_error { + return Err(e); + } + if missing.is_empty() { + Ok(()) + } else { + Err(CliError::not_found(format!("Issue(s) not found: {}", missing.join(", "))).into()) + } } async fn fetch_issue_meta(client: &LinearClient, issue_id: &str) -> Result { @@ -335,6 +368,34 @@ mod tests { "badtitle" ); } + + #[test] + fn comments_status_ok_when_nothing_failed() { + assert!(comments_status(None, &[]).is_ok()); + } + + #[test] + fn comments_status_notfound_when_only_missing() { + let err = comments_status(None, &["LIN-9".to_string()]).unwrap_err(); + assert_eq!(err.downcast_ref::().expect("CliError").code(), 2); + } + + #[test] + fn comments_status_preserves_real_error_over_missing() { + // A real fetch error keeps its kind (rate-limited => 4), not NotFound(2). + let err = comments_status( + Some( + CliError::rate_limited("429") + .with_retry_after(Some(5)) + .into(), + ), + &["LIN-9".to_string()], + ) + .unwrap_err(); + let cli = err.downcast_ref::().expect("CliError"); + assert_eq!(cli.code(), 4); + assert_eq!(cli.retry_after, Some(5)); + } } async fn update_comment(id: &str, body: &str, output: &OutputOptions) -> Result<()> { diff --git a/src/commands/issues.rs b/src/commands/issues.rs index ff84fe9..b092ecc 100644 --- a/src/commands/issues.rs +++ b/src/commands/issues.rs @@ -11,6 +11,7 @@ use crate::api::{ }; use crate::cache::CacheOptions; use crate::display_options; +use crate::error::{self, CliError}; use crate::input::read_ids_from_stdin; use crate::output::{ ensure_non_empty, filter_values, print_json, print_json_owned, sort_values, OutputOptions, @@ -804,6 +805,10 @@ async fn get_issues( .collect() .await; + // Any missing/errored issue exits non-zero; the issues found are still + // printed. Computed before printing so `results` can be consumed below. + let status = multi_get_status(&results); + // JSON output: array of issues if output.is_json() || output.has_template() { let issues: Vec<_> = results @@ -820,7 +825,7 @@ async fn get_issues( }) .collect(); print_json_owned(serde_json::json!(issues), output)?; - return Ok(()); + return status; } // Table output @@ -845,7 +850,34 @@ async fn get_issues( } } - Ok(()) + status +} + +/// Aggregate exit status for a multi-issue fetch. A real fetch error takes +/// priority and keeps its kind (e.g. RateLimited stays retryable); only a `null` +/// issue node maps to NotFound. +fn multi_get_status(results: &[(String, Result)]) -> Result<()> { + if let Some((id, e)) = results.iter().find_map(|(id, r)| match r { + Err(e) => Some((id, e)), + Ok(_) => None, + }) { + return Err(error::rewrap_with_message( + e, + format!("Error fetching {id}: {e}"), + )); + } + + let missing: Vec<&str> = results + .iter() + .filter(|(_, r)| matches!(r, Ok(data) if data["data"]["issue"].is_null())) + .map(|(id, _)| id.as_str()) + .collect(); + + if missing.is_empty() { + Ok(()) + } else { + Err(CliError::not_found(format!("Issue(s) not found: {}", missing.join(", "))).into()) + } } async fn open_issue(id: &str) -> Result<()> { @@ -2266,6 +2298,47 @@ async fn transfer_issue(id: &str, team: &str) -> Result<()> { mod tests { use super::*; + #[test] + fn multi_get_status_ok_when_all_found() { + let results = vec![( + "LIN-1".to_string(), + Ok(json!({ "data": { "issue": { "id": "x" } } })), + )]; + assert!(multi_get_status(&results).is_ok()); + assert!(multi_get_status(&[]).is_ok()); + } + + #[test] + fn multi_get_status_notfound_when_issue_null() { + let results = vec![( + "LIN-9".to_string(), + Ok(json!({ "data": { "issue": null } })), + )]; + let err = multi_get_status(&results).unwrap_err(); + assert_eq!(err.downcast_ref::().expect("CliError").code(), 2); + } + + #[test] + fn multi_get_status_preserves_rate_limited_kind_over_notfound() { + // A 429 must stay RateLimited(4), not collapse to NotFound(2). + let results: Vec<(String, Result)> = vec![ + ( + "LIN-1".to_string(), + Ok(json!({ "data": { "issue": null } })), + ), + ( + "LIN-2".to_string(), + Err(CliError::rate_limited("429") + .with_retry_after(Some(3)) + .into()), + ), + ]; + let err = multi_get_status(&results).unwrap_err(); + let cli = err.downcast_ref::().expect("CliError"); + assert_eq!(cli.code(), 4); + assert_eq!(cli.retry_after, Some(3)); + } + #[test] fn test_format_history_state_change() { let entry = serde_json::json!({ diff --git a/src/error.rs b/src/error.rs index f48def1..0b59052 100644 --- a/src/error.rs +++ b/src/error.rs @@ -72,6 +72,74 @@ impl CliError { self.retry_after = retry_after; self } + + /// Classify a GraphQL `errors` array into a `CliError` with a precise exit code. + /// + /// Linear reports application-level failures as HTTP 200 with an `errors` + /// array, so the real signal lives in the first error's `extensions` + /// (`statusCode` / `code`) or its message text rather than the transport + /// status. The message stays "GraphQL error" so `Display` still appends the + /// underlying error messages. + pub fn from_graphql_errors(errors: &Value) -> Self { + let first = errors.as_array().and_then(|arr| arr.first()); + let kind = first.map_or(ErrorKind::General, Self::classify_graphql_error); + let mut err = Self::new(kind, "GraphQL error").with_details(errors.clone()); + if kind == ErrorKind::RateLimited { + let retry_after = first + .and_then(|e| e.get("extensions")) + .and_then(|ext| ext.get("retryAfter")) + .and_then(Value::as_u64); + err = err.with_retry_after(retry_after); + } + err + } + + fn classify_graphql_error(error: &Value) -> ErrorKind { + let ext = error.get("extensions"); + + // 1. Numeric status code carried in extensions, when present. + if let Some(status) = ext + .and_then(|e| e.get("statusCode")) + .and_then(Value::as_u64) + { + match status { + 401 | 403 => return ErrorKind::Auth, + 404 => return ErrorKind::NotFound, + 429 => return ErrorKind::RateLimited, + _ => {} + } + } + + // 2. Symbolic code string (e.g. AUTHENTICATION_ERROR, RATELIMITED). + if let Some(code) = ext.and_then(|e| e.get("code")).and_then(Value::as_str) { + let code = code.to_ascii_uppercase(); + if code.contains("AUTH") { + return ErrorKind::Auth; + } + if code.contains("RATELIM") { + return ErrorKind::RateLimited; + } + if code.contains("NOT_FOUND") { + return ErrorKind::NotFound; + } + } + + // 3. Message text — Linear reports "Entity not found" with statusCode 400. + let msg = error + .get("message") + .and_then(Value::as_str) + .or_else(|| { + ext.and_then(|e| e.get("userPresentableMessage")) + .and_then(Value::as_str) + }) + .unwrap_or(""); + let lower = msg.to_ascii_lowercase(); + if lower.contains("not found") || lower.contains("could not find") { + return ErrorKind::NotFound; + } + + ErrorKind::General + } } impl fmt::Display for CliError { @@ -110,6 +178,30 @@ impl fmt::Display for CliError { impl Error for CliError {} +/// Re-wrap `source` with a new human-readable `message`, preserving the original +/// [`CliError`]'s `kind` / `details` / `retry_after` when `source` is (or wraps, +/// via `anyhow` context) a `CliError`. +/// +/// This is the canonical place commands re-message an error without losing its +/// exit-code semantics: a wrapped rate-limited error stays [`ErrorKind::RateLimited`] +/// (exit 4) with its `retry_after`, instead of being flattened to a generic +/// error. When `source` is not a `CliError`, a generic error carrying `message` +/// is returned — callers should embed the original cause in `message` so the +/// string-based exit-code heuristics still apply. +pub fn rewrap_with_message(source: &anyhow::Error, message: impl Into) -> anyhow::Error { + let message = message.into(); + match source.downcast_ref::() { + Some(cli) => { + let mut wrapped = CliError::new(cli.kind, message).with_retry_after(cli.retry_after); + if let Some(details) = &cli.details { + wrapped = wrapped.with_details(details.clone()); + } + wrapped.into() + } + None => anyhow::anyhow!("{message}"), + } +} + #[cfg(test)] mod tests { use super::*; @@ -199,4 +291,128 @@ mod tests { assert_eq!(err.code(), 2); assert_eq!(err.kind, ErrorKind::NotFound); } + + #[test] + fn rewrap_preserves_kind_and_retry_after() { + let source: anyhow::Error = CliError::rate_limited("429") + .with_retry_after(Some(9)) + .into(); + let wrapped = rewrap_with_message(&source, "while fetching LIN-1"); + let cli = wrapped + .downcast_ref::() + .expect("CliError preserved"); + assert_eq!(cli.kind, ErrorKind::RateLimited); + assert_eq!(cli.code(), 4); + assert_eq!(cli.retry_after, Some(9)); + assert_eq!(cli.message, "while fetching LIN-1"); + } + + #[test] + fn rewrap_preserves_details() { + let source: anyhow::Error = CliError::general("boom") + .with_details(json!({ "field": "value" })) + .into(); + let wrapped = rewrap_with_message(&source, "new message"); + let cli = wrapped + .downcast_ref::() + .expect("CliError preserved"); + assert_eq!(cli.details, Some(json!({ "field": "value" }))); + } + + #[test] + fn rewrap_plain_error_keeps_message() { + let source = anyhow::anyhow!("not found"); + let wrapped = rewrap_with_message(&source, "Failed to resolve 'x': not found"); + assert!(wrapped.downcast_ref::().is_none()); + assert_eq!(wrapped.to_string(), "Failed to resolve 'x': not found"); + } + + #[test] + fn rewrap_finds_cli_error_behind_anyhow_context() { + use anyhow::Context; + // A CliError wrapped in an anyhow context layer must still be found, + // so re-messaging across command boundaries preserves the exit code. + let source = Err::<(), _>(CliError::auth("401")) + .context("resolving user") + .unwrap_err(); + let wrapped = rewrap_with_message(&source, "Failed to resolve user 'me'"); + assert_eq!( + wrapped + .downcast_ref::() + .expect("CliError preserved") + .code(), + 3 + ); + } + + #[test] + fn graphql_entity_not_found_maps_to_not_found() { + // Linear returns "Entity not found" as HTTP 200 + errors array with + // statusCode 400 (NOT 404), so classification must fall through to the + // message-text check. This is the common `i get ` path. + let errors = json!([{ + "message": "Entity not found: Issue", + "extensions": { + "code": "INPUT_ERROR", + "statusCode": 400, + "type": "invalid input", + "userError": true, + "userPresentableMessage": "Could not find referenced Issue." + } + }]); + let err = CliError::from_graphql_errors(&errors); + assert_eq!(err.kind, ErrorKind::NotFound); + assert_eq!(err.code(), 2); + assert_eq!(err.to_string(), "GraphQL error: Entity not found: Issue"); + } + + #[test] + fn graphql_auth_error_maps_to_auth() { + let errors = json!([{ + "message": "Authentication required", + "extensions": { "code": "AUTHENTICATION_ERROR" } + }]); + let err = CliError::from_graphql_errors(&errors); + assert_eq!(err.kind, ErrorKind::Auth); + assert_eq!(err.code(), 3); + } + + #[test] + fn graphql_rate_limit_maps_to_rate_limited_with_retry_after() { + let errors = json!([{ + "message": "Too many requests", + "extensions": { "code": "RATELIMITED", "retryAfter": 30 } + }]); + let err = CliError::from_graphql_errors(&errors); + assert_eq!(err.kind, ErrorKind::RateLimited); + assert_eq!(err.code(), 4); + assert_eq!(err.retry_after, Some(30)); + } + + #[test] + fn graphql_status_code_takes_precedence() { + // A 403 in extensions classifies as Auth even without a code string. + let errors = json!([{ + "message": "denied", + "extensions": { "statusCode": 403 } + }]); + assert_eq!(CliError::from_graphql_errors(&errors).code(), 3); + } + + #[test] + fn graphql_generic_error_stays_general() { + let errors = json!([{ "message": "Field 'foo' is invalid" }]); + let err = CliError::from_graphql_errors(&errors); + assert_eq!(err.kind, ErrorKind::General); + assert_eq!(err.code(), 1); + } + + #[test] + fn graphql_empty_errors_stays_general() { + let errors = json!([]); + assert_eq!( + CliError::from_graphql_errors(&errors).kind, + ErrorKind::General + ); + } } diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index a2aa9ce..a10f033 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -2142,3 +2142,70 @@ fn test_yes_flag_works_with_subcommand() { "--yes flag should not interfere with subcommand parsing" ); } + +// ---- Exit-code / JSON-error contract (agent ergonomics) ---- +// Offline checks via the `bulk label` empty-issues path (a failure reached +// before any network/config access). Credential-dependent paths are covered by +// the unit tests in src/commands/* and by live E2E. + +/// Run the CLI with an isolated, empty config home and no ambient credentials. +/// Each call gets a unique HOME so parallel tests never share state. +fn run_cli_isolated(args: &[&str]) -> (i32, String, String) { + use std::sync::atomic::{AtomicUsize, Ordering}; + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let tmp = + std::env::temp_dir().join(format!("linear-cli-isolated-{}-{}", std::process::id(), n)); + let _ = std::fs::create_dir_all(&tmp); + let output = Command::new(env!("CARGO_BIN_EXE_linear-cli")) + .args(args) + .env("HOME", &tmp) + .env("XDG_CONFIG_HOME", &tmp) + .env_remove("LINEAR_API_KEY") + .env_remove("LINEAR_CLI_PROFILE") + .output() + .expect("Failed to execute command"); + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let code = output.status.code().unwrap_or(-1); + + (code, stdout, stderr) +} + +#[test] +fn test_failing_command_exits_nonzero() { + // `bulk label` with no issues is a logical failure: must not exit 0. + let (code, _stdout, stderr) = run_cli_isolated(&["bulk", "label", "Bug"]); + assert_ne!( + code, 0, + "a failing command must exit non-zero; stderr={stderr}" + ); + assert!( + (1..=4).contains(&code), + "exit code should be a documented error code (1-4), got {code}" + ); +} + +#[test] +fn test_json_error_body_emitted_on_stderr_not_stdout() { + // stdout is empty on this path; the results-vs-error stream split is + // unit-tested in src/commands/{bulk,issues,comments}.rs. + let (code, stdout, stderr) = run_cli_isolated(&["bulk", "label", "Bug", "--output", "json"]); + assert_ne!(code, 0, "failing command must exit non-zero"); + assert!( + stderr.contains("\"error\":true"), + "stderr should carry the JSON error body, got: {stderr}" + ); + assert!( + !stdout.contains("\"error\":true"), + "stdout must not carry a second JSON error body, got: {stdout}" + ); +} + +#[test] +fn test_successful_command_exits_zero() { + let (code, stdout, _stderr) = run_cli_isolated(&["--version"]); + assert_eq!(code, 0, "a successful command must exit 0"); + assert!(!stdout.is_empty()); +}