From bd3a4e4d5462194937067d914c9fd04fa6fe4af6 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:25:53 -0700 Subject: [PATCH 01/19] feat(support): add support report command Files a ticket via POST {app_url}/v1/support/issues: compose with -m/--subject or $EDITOR, attach the active workspace and diagnostic context, optionally attach logs, and retry once on a connection error or 5xx using a stable idempotency key. --- src/cli.rs | 7 + src/client.rs | 1 + src/client/jwt.rs | 2 +- src/client/support.rs | 309 ++++++++++++ src/commands.rs | 1 + src/commands/support.rs | 1032 +++++++++++++++++++++++++++++++++++++++ src/main.rs | 24 + src/util.rs | 40 ++ 8 files changed, 1415 insertions(+), 1 deletion(-) create mode 100644 src/client/support.rs create mode 100644 src/commands/support.rs diff --git a/src/cli.rs b/src/cli.rs index f84ae6f7..fbc5bdc1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -5,6 +5,7 @@ use crate::commands::jobs::JobsCommands; use crate::commands::query::QueryCommands; use crate::commands::search::SearchCommands; use crate::commands::skill::SkillCommands; +use crate::commands::support::SupportCommands; use crate::commands::workspace::WorkspaceCommands; use clap::Subcommand; @@ -143,6 +144,12 @@ pub enum Commands { #[command(subcommand)] command: ManageCommands, }, + + /// Get help — file a support ticket with the HotData team + Support { + #[command(subcommand)] + command: SupportCommands, + }, } /// Subcommands for `hotdata manage` — account and CLI utilities. diff --git a/src/client.rs b/src/client.rs index c94d736f..2be8861c 100644 --- a/src/client.rs +++ b/src/client.rs @@ -3,3 +3,4 @@ pub mod ingest; pub mod jwt; pub mod raw_http; pub mod sdk; +pub mod support; diff --git a/src/client/jwt.rs b/src/client/jwt.rs index 03f6479f..04d2b7d8 100644 --- a/src/client/jwt.rs +++ b/src/client/jwt.rs @@ -121,7 +121,7 @@ fn session_from_response( } } -fn oauth_base(profile: &config::ProfileConfig) -> String { +pub(crate) fn oauth_base(profile: &config::ProfileConfig) -> String { // DOT (`/o/authorize/`, `/o/token/`, …) is mounted on the webapp // (app_url), not the API. The api_url host typically only serves // the `/v1` runtimedb routes. diff --git a/src/client/support.rs b/src/client/support.rs new file mode 100644 index 00000000..0ca3eedb --- /dev/null +++ b/src/client/support.rs @@ -0,0 +1,309 @@ +//! Raw-HTTP client for `POST {app_url}/v1/support/issues`. +//! +//! Lives on the webapp (`app_url`), not the API gateway (`api_url`) — the +//! same host `jwt::oauth_base` resolves for `/v1/auth/token`. No SDK +//! operation exists for this route yet, so it rides the hand-rolled +//! `reqwest::blocking` seam alongside the token endpoints. + +use crate::client::jwt; +use crate::config; +use crate::util; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::time::Duration; + +/// Retry a failed POST once after this long — long enough that a transient +/// blip (a dropped connection, a mid-deploy 502) has usually cleared. The +/// idempotency key carried on the request is what makes a retried create safe. +const RETRY_DELAY: Duration = Duration::from_secs(2); + +#[derive(Debug, Serialize)] +pub struct SupportIssueRequest { + pub subject: String, + pub body: String, + pub kind: String, + pub severity: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub workspace_public_id: Option, + #[serde(skip_serializing_if = "BTreeMap::is_empty")] + pub context: BTreeMap, + #[serde(skip_serializing_if = "Option::is_none")] + pub logs: Option, + pub idempotency_key: String, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct SupportIssue { + pub public_id: String, + pub status: String, + pub subject: String, + pub kind: String, + pub severity: String, + pub workspace_public_id: Option, + pub created_at: String, +} + +#[derive(Deserialize)] +struct SupportIssueEnvelope { + issue: SupportIssue, +} + +/// A typed error from the support-issue call, mirroring the shape other raw +/// clients (`client::ingest`) use so callers can pattern-match on it. +#[derive(Debug)] +pub enum SupportError { + /// Non-2xx response; `body` is the server's (unredacted) response text. + Http { status: u16, body: String }, + /// Transport/connection failure. + Connection(String), + /// 2xx whose body didn't match the expected envelope. + Decode(String), + /// Could not resolve a bearer token (`jwt::ensure_access_token` failed). + Auth(String), +} + +impl SupportError { + /// Worth a single retry: a connection never completed, or the server + /// itself failed (5xx). A 4xx is the server telling us the request is + /// wrong — retrying it verbatim would just fail again. + fn is_retryable(&self) -> bool { + matches!(self, SupportError::Connection(_)) + || matches!(self, SupportError::Http { status, .. } if *status >= 500) + } +} + +fn url(profile: &config::ProfileConfig) -> String { + format!("{}/v1/support/issues", jwt::oauth_base(profile)) +} + +fn send_once( + client: &reqwest::blocking::Client, + profile: &config::ProfileConfig, + token: &str, + req: &SupportIssueRequest, +) -> Result<(SupportIssue, bool), SupportError> { + let body = serde_json::to_value(req).expect("SupportIssueRequest serializes"); + let builder = client + .post(url(profile)) + .header("Authorization", format!("Bearer {token}")) + .header( + "User-Agent", + concat!("hotdata-cli/", env!("CARGO_PKG_VERSION")), + ) + .json(&body); + let (status, body_text) = util::send_debug(client, builder, Some(&body)) + .map_err(|e| SupportError::Connection(e.to_string()))?; + if !status.is_success() { + return Err(SupportError::Http { + status: status.as_u16(), + body: body_text, + }); + } + // 200 is the idempotent-replay shape; 202 the freshly-queued one — same + // envelope either way, so only the status tells them apart. + let replay = status.as_u16() == 200; + let parsed: SupportIssueEnvelope = + serde_json::from_str(&body_text).map_err(|e| SupportError::Decode(e.to_string()))?; + Ok((parsed.issue, replay)) +} + +/// File a support issue. Retries once, after [`RETRY_DELAY`], on a connection +/// error or 5xx — never on a 4xx — reusing the same `idempotency_key` so a +/// retried create can't double-file. +pub fn post_support_issue( + profile: &config::ProfileConfig, + req: &SupportIssueRequest, +) -> Result<(SupportIssue, bool), SupportError> { + post_support_issue_with_delay(profile, req, RETRY_DELAY) +} + +fn post_support_issue_with_delay( + profile: &config::ProfileConfig, + req: &SupportIssueRequest, + retry_delay: Duration, +) -> Result<(SupportIssue, bool), SupportError> { + // Same trust filter as sdk::Api / client::ingest: an empty or template + // key must fall through to the session JWT, not ship as a bearer. + let api_key_fallback = profile + .api_key + .as_deref() + .filter(|k| !k.is_empty() && *k != "PLACEHOLDER"); + let token = jwt::ensure_access_token(profile, api_key_fallback).map_err(SupportError::Auth)?; + let client = crate::client::raw_http::build_http_client(); + + match send_once(&client, profile, &token, req) { + Ok(ok) => Ok(ok), + Err(e) if e.is_retryable() => { + std::thread::sleep(retry_delay); + send_once(&client, profile, &token, req) + } + Err(e) => Err(e), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{ApiUrl, AppUrl, ProfileConfig, test_helpers::with_temp_config_dir}; + + /// A profile with an api_key set, so every test here resolves a bearer + /// with zero network calls (no session mint/refresh to mock separately). + fn mock_profile(url: &str) -> ProfileConfig { + ProfileConfig { + api_key: Some("hd_test_key".to_string()), + app_url: AppUrl(Some(url.to_string())), + api_url: ApiUrl(Some(url.to_string())), + ..Default::default() + } + } + + fn req(idempotency_key: &str) -> SupportIssueRequest { + SupportIssueRequest { + subject: "Query timing out".into(), + body: "Queries against my workspace have been hanging for an hour.".into(), + kind: "bug".into(), + severity: "high".into(), + workspace_public_id: Some("work_abc".into()), + context: BTreeMap::from([("cli_version".to_string(), "0.31.0".to_string())]), + logs: None, + idempotency_key: idempotency_key.to_string(), + } + } + + #[test] + fn happy_path_202_is_not_a_replay() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .match_header("Authorization", "Bearer hd_test_key") + .match_header("content-type", "application/json") + .match_body(mockito::Matcher::PartialJson(serde_json::json!({ + "subject": "Query timing out", + "kind": "bug", + "severity": "high", + "workspace_public_id": "work_abc", + "idempotency_key": "fixed-key-1", + }))) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_1","status":"queued","subject":"Query timing out","kind":"bug","severity":"high","workspace_public_id":"work_abc","created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + let profile = mock_profile(&server.url()); + let mut r = req("fixed-key-1"); + r.workspace_public_id = Some("work_abc".into()); + let (issue, replay) = post_support_issue_with_delay(&profile, &r, Duration::ZERO).unwrap(); + m.assert(); + assert_eq!(issue.public_id, "supp_1"); + assert_eq!(issue.status, "queued"); + assert!(!replay); + } + + #[test] + fn replay_200_is_reported_as_replay() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_2","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + let profile = mock_profile(&server.url()); + let (issue, replay) = + post_support_issue_with_delay(&profile, &req("fixed-key-2"), Duration::ZERO).unwrap(); + m.assert(); + assert_eq!(issue.public_id, "supp_2"); + assert!(replay); + } + + #[test] + fn server_500_then_202_retries_once_with_same_key() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let key_matcher = + || mockito::Matcher::PartialJson(serde_json::json!({"idempotency_key": "fixed-key-3"})); + // Registered first: consumed by request #1 (default expectation is + // satisfied after a single hit, so request #2 falls through to the + // mock below). + let fail = server + .mock("POST", "/v1/support/issues") + .match_body(key_matcher()) + .with_status(500) + .expect(1) + .create(); + let ok = server + .mock("POST", "/v1/support/issues") + .match_body(key_matcher()) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_3","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .expect(1) + .create(); + + let profile = mock_profile(&server.url()); + let (issue, replay) = + post_support_issue_with_delay(&profile, &req("fixed-key-3"), Duration::ZERO).unwrap(); + fail.assert(); + ok.assert(); + assert_eq!(issue.public_id, "supp_3"); + assert!(!replay); + } + + #[test] + fn connection_error_retries_once_then_gives_up() { + // Nothing listens on port 1 for either attempt — both fail at the + // transport level, and the caller sees a Connection error, not a hang. + let (_tmp, _guard) = with_temp_config_dir(); + let profile = mock_profile("http://127.0.0.1:1"); + let err = post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap_err(); + assert!(matches!(err, SupportError::Connection(_))); + } + + #[test] + fn client_error_is_not_retried() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .with_status(422) + .with_body(r#"{"error":"subject_required"}"#) + // Exactly one hit expected — a second request here fails the test. + .expect(1) + .create(); + + let profile = mock_profile(&server.url()); + let err = post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap_err(); + m.assert(); + assert!(matches!(err, SupportError::Http { status: 422, .. })); + } + + #[test] + fn uses_app_url_not_api_url() { + // app_url and api_url point at different hosts; the request must hit + // the webapp host (app_url), never the api_url one. + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_4","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + let mut profile = mock_profile(&server.url()); + profile.api_url = ApiUrl(Some("http://127.0.0.1:1".to_string())); + post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap(); + m.assert(); + } +} diff --git a/src/commands.rs b/src/commands.rs index 045de810..204020a7 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -17,6 +17,7 @@ pub mod run; pub mod schema_form; pub mod search; pub mod skill; +pub mod support; pub mod tables; pub mod update; pub mod usage; diff --git a/src/commands/support.rs b/src/commands/support.rs new file mode 100644 index 00000000..80d2863e --- /dev/null +++ b/src/commands/support.rs @@ -0,0 +1,1032 @@ +//! `hotdata support report` — file a ticket through the webapp's support +//! intake (`POST {app_url}/v1/support/issues`). `client::support` owns the +//! HTTP call; this module owns composing the request and rendering the +//! result. +//! +//! Validation (`build_request`, and everything it calls) is deliberately +//! `Result`-returning rather than `eprintln!` + `process::exit` directly, so +//! every "reject before any HTTP call" path is a plain function call in +//! tests, not a process-terminating one. + +use crate::client; +use crate::client::support::{SupportError, SupportIssue, SupportIssueRequest}; +use crate::config; +use crate::util; +use serde::Serialize; +use std::collections::BTreeMap; + +const MAX_LOGS_BYTES: usize = 256 * 1024; +const MAX_CONTEXT_VALUE_CHARS: usize = 500; +const MAX_USER_CONTEXT_PAIRS: usize = 20; +/// The one non-error exit message ("aborted, nothing sent") shares the error +/// channel (`Result<_, String>`) with real validation failures; this marker +/// is how the top-level caller tells them apart to skip the "error: " prefix. +const ABORTED: &str = "aborted, nothing sent"; + +/// Subcommands for `hotdata support`. +#[derive(clap::Subcommand)] +pub enum SupportCommands { + /// File a support ticket + Report { + /// Report body. Omit to compose in $EDITOR (TTY only) + #[arg(short = 'm', long = "message")] + message: Option, + + /// Subject line (<= 200 chars). Required with -m; when composing in + /// $EDITOR the first non-comment line of the file is the subject + #[arg(long)] + subject: Option, + + /// Kind of report + #[arg(long, default_value = "other", value_parser = ["bug", "question", "billing", "feature", "account", "other"])] + kind: String, + + /// Severity + #[arg(long, default_value = "medium", value_parser = ["urgent", "high", "medium", "low"])] + severity: String, + + /// Workspace to attach (defaults to the active workspace from config) + #[arg(short = 'w', long = "workspace-id", conflicts_with = "no_workspace")] + workspace_id: Option, + + /// Do not attach a workspace + #[arg(long)] + no_workspace: bool, + + /// Attach a text file as logs ('-' reads stdin); client-side cap 256 KiB + #[arg(long)] + logs: Option, + + /// Extra context pair KEY=VALUE, repeatable (max 20) + #[arg(long = "context")] + context: Vec, + + /// Output format + #[arg(long = "output", short = 'o', default_value = "table", value_parser = ["table", "json", "yaml"])] + output: String, + }, +} + +#[allow(clippy::too_many_arguments)] +pub fn report( + message: Option, + subject: Option, + kind: String, + severity: String, + workspace_id: Option, + no_workspace: bool, + logs_path: Option, + context_pairs: Vec, + output: &str, +) { + let profile = config::load("default").unwrap_or_else(|e| { + eprintln!("{e}"); + std::process::exit(1); + }); + report_with_profile( + &profile, + message, + subject, + kind, + severity, + workspace_id, + no_workspace, + logs_path, + context_pairs, + output, + ); +} + +#[allow(clippy::too_many_arguments)] +fn report_with_profile( + profile: &config::ProfileConfig, + message: Option, + subject: Option, + kind: String, + severity: String, + workspace_id: Option, + no_workspace: bool, + logs_path: Option, + context_pairs: Vec, + output: &str, +) { + let (req, workspace_id) = build_request( + profile, + message, + subject, + kind, + severity, + workspace_id, + no_workspace, + logs_path, + context_pairs, + ) + .unwrap_or_else(|msg| { + if msg == ABORTED { + eprintln!("{msg}"); + } else { + eprintln!("error: {msg}"); + } + std::process::exit(1); + }); + + match client::support::post_support_issue(profile, &req) { + Ok((issue, replay)) => print_success(&issue, replay, output), + Err(e) => handle_error(&e, workspace_id.as_deref()), + } +} + +/// Resolve the workspace, compose the report text, build context, and load +/// logs — everything that can be rejected before an HTTP call is ever made. +/// Returns the request plus the resolved workspace id (needed to render a +/// `workspace_not_found` error message later). +#[allow(clippy::too_many_arguments)] +fn build_request( + profile: &config::ProfileConfig, + message: Option, + subject: Option, + kind: String, + severity: String, + workspace_id: Option, + no_workspace: bool, + logs_path: Option, + context_pairs: Vec, +) -> Result<(SupportIssueRequest, Option), String> { + let (workspace_id, workspace_locked) = + resolve_optional_workspace(profile, workspace_id, no_workspace)?; + let (subject, body) = compose(message, subject)?; + + let mut context = default_context(profile, workspace_locked); + merge_user_context(&mut context, &context_pairs)?; + + let logs = match logs_path { + Some(path) => Some(load_logs(&path)?), + None => None, + }; + + let req = SupportIssueRequest { + subject, + body, + kind, + severity, + workspace_public_id: workspace_id.clone(), + context, + logs, + idempotency_key: generate_idempotency_key(), + }; + Ok((req, workspace_id)) +} + +/// Resolve the workspace to attach. Unlike `main::resolve_workspace`, an +/// unconfigured default is not an error here — proceed with none rather than +/// block the one caller most likely to have a broken setup. Still honors the +/// `HOTDATA_WORKSPACE` lock: an explicit `--workspace-id` that disagrees with +/// it is still rejected, same as every other command. +/// +/// Returns `(workspace_id, locked)`; `locked` feeds the `workspace_locked` +/// context key. +fn resolve_optional_workspace( + profile: &config::ProfileConfig, + provided: Option, + no_workspace: bool, +) -> Result<(Option, bool), String> { + if no_workspace { + return Ok((None, false)); + } + if let Ok(ws) = std::env::var("HOTDATA_WORKSPACE") { + if let Some(flag) = &provided + && flag != &ws + { + return Err(format!( + "cannot override workspace -- locked by HOTDATA_WORKSPACE environment variable ({ws})" + )); + } + return Ok((Some(ws), true)); + } + if let Some(id) = provided { + return Ok((Some(id), false)); + } + Ok((client::credentials::default_workspace_id(profile), false)) +} + +/// Produce (subject, body) from `-m`/`--subject`, or by composing in +/// `$EDITOR` when neither is usable. The abort case (empty compose) is +/// signaled via the literal [`ABORTED`] string so the caller skips the +/// "error: " prefix on it. +fn compose(message: Option, subject: Option) -> Result<(String, String), String> { + if let Some(body) = message { + let Some(subject) = subject else { + return Err("--subject is required when using -m/--message".to_string()); + }; + return Ok((subject, body)); + } + + if !util::is_interactive() { + return Err( + "stdin is not a TTY; pass -m/--message and --subject to file a report non-interactively" + .to_string(), + ); + } + + let template = format!( + "{}\n\n\ + # Lines starting with '#' are ignored. First non-comment line is the subject,\n\ + # the rest is the report body. Save and quit to send; empty to abort.\n", + subject.unwrap_or_default() + ); + let edited = util::open_editor(&template)?; + parse_composed(&edited).ok_or_else(|| ABORTED.to_string()) +} + +/// Pure parse of an edited compose file: strip `#`-comment lines, take the +/// first non-blank remaining line as the subject and everything after as the +/// body. `None` when either comes up empty — the abort case. +fn parse_composed(text: &str) -> Option<(String, String)> { + let mut lines = text.lines().filter(|l| !l.trim_start().starts_with('#')); + let subject = loop { + match lines.next() { + Some(l) if l.trim().is_empty() => continue, + Some(l) => break l.trim().to_string(), + None => return None, + } + }; + let body: String = lines.collect::>().join("\n").trim().to_string(); + if subject.is_empty() || body.is_empty() { + return None; + } + Some((subject, body)) +} + +/// Truncate to `max` chars (not bytes), respecting UTF-8 boundaries. +fn truncate_chars(s: &str, max: usize) -> String { + s.chars().take(max).collect() +} + +fn default_context( + profile: &config::ProfileConfig, + workspace_locked: bool, +) -> BTreeMap { + let mut context = BTreeMap::new(); + context.insert( + "cli_version".to_string(), + env!("CARGO_PKG_VERSION").to_string(), + ); + context.insert( + "os".to_string(), + format!("{}/{}", std::env::consts::OS, std::env::consts::ARCH), + ); + context.insert("api_url".to_string(), profile.api_url.to_string()); + // No profile-name context key: ProfileConfig carries no name of its own + // (it's looked up by an external string key, e.g. "default"), so there is + // nothing to report here — the spec's "skip otherwise". + context.insert("workspace_locked".to_string(), workspace_locked.to_string()); + context.insert( + "no_input".to_string(), + (!util::is_interactive()).to_string(), + ); + context +} + +/// Merge user `--context KEY=VALUE` pairs over the defaults (user wins). +/// Rejects a malformed pair, an empty key, or too many pairs — before any of +/// them are ever sent. +fn merge_user_context( + context: &mut BTreeMap, + pairs: &[String], +) -> Result<(), String> { + if pairs.len() > MAX_USER_CONTEXT_PAIRS { + return Err(format!( + "too many --context pairs ({} given, max {MAX_USER_CONTEXT_PAIRS})", + pairs.len() + )); + } + for pair in pairs { + let Some((key, value)) = pair.split_once('=') else { + return Err(format!("--context '{pair}' is not in KEY=VALUE form")); + }; + if key.is_empty() { + return Err(format!("--context '{pair}' has an empty key")); + } + context.insert( + key.to_string(), + truncate_chars(value, MAX_CONTEXT_VALUE_CHARS), + ); + } + Ok(()) +} + +/// Read `--logs`' file (or stdin for `-`), enforcing the 256 KiB client-side +/// cap before any HTTP call, then apply the one client-side redaction the +/// spec asks for: mask the value on a line that looks like an `Authorization:` +/// header. The server does the real redaction; this just keeps an obvious +/// credential out of `--debug` output and off the wire in the clear case +/// where the user pasted a raw curl invocation into their saved log. +fn load_logs(path: &str) -> Result { + let bytes = if path == "-" { + use std::io::Read; + let mut buf = Vec::new(); + std::io::stdin() + .read_to_end(&mut buf) + .map_err(|e| format!("reading stdin for --logs: {e}"))?; + buf + } else { + std::fs::read(path).map_err(|e| format!("reading --logs file '{path}': {e}"))? + }; + if bytes.len() > MAX_LOGS_BYTES { + return Err(format!( + "--logs is {} bytes; the client-side cap is {MAX_LOGS_BYTES} bytes (256 KiB)", + bytes.len() + )); + } + Ok(redact_logs(&String::from_utf8_lossy(&bytes))) +} + +fn redact_logs(text: &str) -> String { + text.lines() + .map(redact_log_line) + .collect::>() + .join("\n") +} + +fn redact_log_line(line: &str) -> String { + let trimmed = line.trim_start(); + let indent = &line[..line.len() - trimmed.len()]; + let Some((name, value)) = trimmed.split_once(':') else { + return line.to_string(); + }; + if !name.eq_ignore_ascii_case("authorization") { + return line.to_string(); + } + let value = value.trim(); + let masked = match value.strip_prefix("Bearer ") { + Some(token) => format!("Bearer {}", util::mask_credential(token)), + None => util::mask_credential(value), + }; + format!("{indent}Authorization: {masked}") +} + +fn generate_idempotency_key() -> String { + use rand::RngCore; + let mut bytes = [0u8; 16]; + rand::thread_rng().fill_bytes(&mut bytes); + bytes.iter().map(|b| format!("{b:02x}")).collect() +} + +#[derive(Serialize)] +struct ReportOutput<'a> { + #[serde(flatten)] + issue: &'a SupportIssue, + replay: bool, +} + +fn print_success(issue: &SupportIssue, replay: bool, output: &str) { + match output { + "json" => println!( + "{}", + serde_json::to_string_pretty(&ReportOutput { issue, replay }).unwrap() + ), + "yaml" => print!( + "{}", + serde_yaml::to_string(&ReportOutput { issue, replay }).unwrap() + ), + "table" => { + use crossterm::style::Stylize; + println!( + "Support request filed: {}", + issue.public_id.as_str().green() + ); + println!("Subject: {}", issue.subject); + let workspace = issue.workspace_public_id.as_deref().unwrap_or("none"); + println!( + "Severity: {} Kind: {} Workspace: {}", + issue.severity, issue.kind, workspace + ); + println!("Replies go to the email on your HotData account."); + if replay { + println!("(already filed; nothing new was sent)"); + } + } + _ => unreachable!(), + } +} + +/// The support endpoint's error envelope is Django-flat (`{"error": +/// "workspace_not_found"}`) — the code IS the string, unlike the nested +/// `{"error": {"code", "message"}}` shape `util::error_code` expects +/// (RuntimeDB/ingest style). Falls back to the nested shape too, in case the +/// webapp ever wraps it that way instead. +fn support_error_code(body: &str) -> Option { + let v: serde_json::Value = serde_json::from_str(body).ok()?; + v["error"] + .as_str() + .map(str::to_string) + .or_else(|| v["error"]["code"].as_str().map(str::to_string)) +} + +/// The human message for a failed call — pure, so error-code mapping is +/// tested directly rather than by spawning the binary. +fn error_message(e: &SupportError, workspace_id: Option<&str>) -> String { + match e { + SupportError::Auth(m) => format!("{m}\nRun 'hotdata auth login' to authenticate."), + SupportError::Connection(m) => format!("connection error: {m}"), + SupportError::Decode(m) => format!("malformed response: {m}"), + SupportError::Http { status, body } => { + let code = support_error_code(body); + match code.as_deref() { + Some("not_found") => { + "support reporting is not enabled for your organization yet; email support@hotdata.dev" + .to_string() + } + Some("rate_limited") => { + "too many reports in the last hour; try again later or email support@hotdata.dev" + .to_string() + } + Some("workspace_not_found") => { + let id = workspace_id.unwrap_or(""); + format!( + "workspace '{id}' not found or not accessible; pass --no-workspace to file without one" + ) + } + Some("missing_authorization") | Some("invalid_api_key") => { + format!( + "{}\nRun 'hotdata auth login' to authenticate.", + util::api_error(body.clone()) + ) + } + Some("body_too_long") => { + "report body is too long (limit 20000 characters)".to_string() + } + Some(code) => format!("support request failed ({status} {code})."), + None => format!("support request failed ({status})."), + } + } + } +} + +fn handle_error(e: &SupportError, workspace_id: Option<&str>) -> ! { + use crossterm::style::Stylize; + + let message = error_message(e, workspace_id); + eprintln!("{}", format!("error: {message}").red()); + if !message.contains("support@hotdata.dev") { + eprintln!( + "{}", + "If this keeps happening, email support@hotdata.dev.".dark_grey() + ); + } + std::process::exit(1); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{ApiUrl, AppUrl, ProfileConfig, test_helpers::with_temp_config_dir}; + + fn mock_profile(url: &str) -> ProfileConfig { + ProfileConfig { + api_key: Some("hd_test_key".to_string()), + app_url: AppUrl(Some(url.to_string())), + api_url: ApiUrl(Some(url.to_string())), + ..Default::default() + } + } + + // --- parse_composed (editor compose, pure) ----------------------------- + + #[test] + fn parse_composed_strips_comments_and_splits_subject_body() { + let text = "\ +My subject line + +# Lines starting with '#' are ignored. First non-comment line is the subject, +# the rest is the report body. Save and quit to send; empty to abort. +First body paragraph. + +Second paragraph. +"; + let (subject, body) = parse_composed(text).unwrap(); + assert_eq!(subject, "My subject line"); + assert_eq!(body, "First body paragraph.\n\nSecond paragraph."); + } + + #[test] + fn parse_composed_empty_subject_aborts() { + assert!(parse_composed("\n# just a comment\n").is_none()); + } + + #[test] + fn parse_composed_subject_only_no_body_aborts() { + assert!(parse_composed("Just a subject\n\n# comment only\n").is_none()); + } + + #[test] + fn parse_composed_ignores_comment_lines_inside_body() { + let text = "Subject\n\nBody line one\n# an ignored comment mid-body\nBody line two"; + let (subject, body) = parse_composed(text).unwrap(); + assert_eq!(subject, "Subject"); + assert_eq!(body, "Body line one\nBody line two"); + } + + // --- compose (message/subject validation) ------------------------------- + + #[test] + fn compose_with_message_but_no_subject_errors() { + let err = compose(Some("body text".to_string()), None).unwrap_err(); + assert!(err.contains("--subject"), "got: {err}"); + } + + #[test] + fn compose_with_message_and_subject_succeeds_without_editor() { + let (subject, body) = + compose(Some("body text".to_string()), Some("Subj".to_string())).unwrap(); + assert_eq!(subject, "Subj"); + assert_eq!(body, "body text"); + } + + #[test] + fn compose_non_interactive_without_message_errors_before_editor() { + // Forcing non-interactive means this returns before ever trying to + // spawn $EDITOR — safe to call from a test. + util::set_no_input(true); + let err = compose(None, None).unwrap_err(); + util::set_no_input(false); + assert!(err.contains("TTY"), "got: {err}"); + assert_ne!(err, ABORTED); + } + + // --- context -------------------------------------------------------------- + + #[test] + fn default_context_carries_the_documented_keys() { + let ctx = default_context(&mock_profile("https://api.example.test"), true); + assert_eq!(ctx.get("cli_version").unwrap(), env!("CARGO_PKG_VERSION")); + assert_eq!(ctx.get("api_url").unwrap(), "https://api.example.test"); + assert_eq!(ctx.get("workspace_locked").unwrap(), "true"); + assert!(ctx.contains_key("os")); + assert!(ctx.contains_key("no_input")); + assert!(!ctx.contains_key("profile")); + } + + #[test] + fn user_context_overrides_default_and_truncates_long_values() { + let mut ctx = default_context(&mock_profile("https://api.example.test"), false); + let long_value = "x".repeat(600); + merge_user_context( + &mut ctx, + &[ + "api_url=overridden".to_string(), + format!("extra={long_value}"), + ], + ) + .unwrap(); + assert_eq!(ctx.get("api_url").unwrap(), "overridden"); + assert_eq!(ctx.get("extra").unwrap().chars().count(), 500); + } + + #[test] + fn user_context_rejects_pair_without_equals() { + let mut ctx = BTreeMap::new(); + let err = merge_user_context(&mut ctx, &["no-equals-sign".to_string()]).unwrap_err(); + assert!(err.contains("no-equals-sign"), "got: {err}"); + assert!(ctx.is_empty(), "a rejected pair must not be applied"); + } + + #[test] + fn user_context_rejects_empty_key() { + let mut ctx = BTreeMap::new(); + let err = merge_user_context(&mut ctx, &["=value".to_string()]).unwrap_err(); + assert!(err.contains("empty key"), "got: {err}"); + } + + #[test] + fn user_context_rejects_more_than_twenty_pairs() { + let pairs: Vec = (0..21).map(|i| format!("k{i}=v")).collect(); + let mut ctx = BTreeMap::new(); + let err = merge_user_context(&mut ctx, &pairs).unwrap_err(); + assert!(err.contains("21"), "got: {err}"); + } + + // --- logs ------------------------------------------------------------------- + + #[test] + fn load_logs_under_cap_reads_and_redacts() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("out.log"); + std::fs::write(&path, "line one\nAuthorization: Bearer supersecrettoken\n").unwrap(); + let out = load_logs(path.to_str().unwrap()).unwrap(); + assert!(out.contains("line one")); + assert!(!out.contains("supersecrettoken")); + } + + #[test] + fn load_logs_over_cap_errors_with_size_and_cap() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("big.log"); + std::fs::write(&path, vec![b'x'; MAX_LOGS_BYTES + 1]).unwrap(); + let err = load_logs(path.to_str().unwrap()).unwrap_err(); + assert!( + err.contains(&(MAX_LOGS_BYTES + 1).to_string()), + "got: {err}" + ); + assert!(err.contains("256 KiB"), "got: {err}"); + } + + #[test] + fn redact_logs_masks_authorization_header_lines_only() { + let input = "GET /v1/foo\nAuthorization: Bearer abcdefghijklmnop\nX-Other: fine\n"; + let out = redact_logs(input); + assert!(out.contains("Authorization: Bearer abcd...mnop")); + assert!(out.contains("X-Other: fine")); + assert!(!out.contains("abcdefghijklmnop")); + } + + #[test] + fn redact_logs_preserves_indent_and_non_bearer_scheme() { + let input = " authorization: hd_abcdefghijkl\n"; + let out = redact_logs(input); + assert_eq!(out, " Authorization: hd_a...ijkl"); + } + + // --- idempotency key -------------------------------------------------------- + + #[test] + fn generate_idempotency_key_is_32_lowercase_hex_chars() { + let key = generate_idempotency_key(); + assert_eq!(key.len(), 32); + assert!( + key.chars() + .all(|c| c.is_ascii_hexdigit() && !c.is_ascii_uppercase()) + ); + } + + #[test] + fn generate_idempotency_key_is_not_constant() { + assert_ne!(generate_idempotency_key(), generate_idempotency_key()); + } + + // --- workspace resolution ----------------------------------------------------- + + #[test] + fn no_workspace_flag_wins_even_with_a_saved_default() { + let profile = ProfileConfig { + workspaces: vec![config::WorkspaceEntry { + public_id: "work_saved".into(), + name: "Saved".into(), + }], + ..Default::default() + }; + let (id, locked) = resolve_optional_workspace(&profile, None, true).unwrap(); + assert_eq!(id, None); + assert!(!locked); + } + + #[test] + fn explicit_workspace_id_is_used_untouched() { + let profile = ProfileConfig::default(); + let (id, locked) = + resolve_optional_workspace(&profile, Some("work_explicit".to_string()), false).unwrap(); + assert_eq!(id.as_deref(), Some("work_explicit")); + assert!(!locked); + } + + #[test] + fn no_configured_default_resolves_to_none_without_erroring() { + // The behavior that differs from main::resolve_workspace: an + // unconfigured profile must not error here. + let profile = ProfileConfig::default(); + let (id, locked) = resolve_optional_workspace(&profile, None, false).unwrap(); + assert_eq!(id, None); + assert!(!locked); + } + + #[test] + fn saved_default_is_used_when_no_flag_given() { + let profile = ProfileConfig { + workspaces: vec![config::WorkspaceEntry { + public_id: "work_saved".into(), + name: "Saved".into(), + }], + ..Default::default() + }; + let (id, locked) = resolve_optional_workspace(&profile, None, false).unwrap(); + assert_eq!(id.as_deref(), Some("work_saved")); + assert!(!locked); + } + + // --- build_request: validation ordering / zero-HTTP guarantees --------------- + + #[test] + fn build_request_context_without_equals_errors_before_any_http_call() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server.mock("POST", "/v1/support/issues").expect(0).create(); + + let err = build_request( + &mock_profile(&server.url()), + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + None, + true, + None, + vec!["broken".to_string()], + ) + .unwrap_err(); + assert!(err.contains("KEY=VALUE"), "got: {err}"); + // build_request never talks to the network at all, so this always + // holds — asserted anyway as the documented guarantee. + m.assert(); + } + + #[test] + fn build_request_logs_over_cap_errors_before_any_http_call() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server.mock("POST", "/v1/support/issues").expect(0).create(); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("big.log"); + std::fs::write(&path, vec![b'x'; MAX_LOGS_BYTES + 1]).unwrap(); + + let err = build_request( + &mock_profile(&server.url()), + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + None, + true, + Some(path.to_str().unwrap().to_string()), + vec![], + ) + .unwrap_err(); + assert!(err.contains("256 KiB"), "got: {err}"); + m.assert(); + } + + #[test] + fn build_request_non_tty_without_message_errors_before_any_http_call() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server.mock("POST", "/v1/support/issues").expect(0).create(); + + util::set_no_input(true); + let err = build_request( + &mock_profile(&server.url()), + None, + None, + "bug".to_string(), + "high".to_string(), + None, + true, + None, + vec![], + ) + .unwrap_err(); + util::set_no_input(false); + assert!(err.contains("TTY"), "got: {err}"); + m.assert(); + } + + #[test] + fn build_request_no_workspace_omits_workspace_public_id() { + let (_tmp, _guard) = with_temp_config_dir(); + let profile = mock_profile("http://127.0.0.1:1"); + let (req, id) = build_request( + &profile, + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + Some("work_ignored".to_string()), + true, + None, + vec![], + ) + .unwrap(); + assert_eq!(req.workspace_public_id, None); + assert_eq!(id, None); + } + + #[test] + fn build_request_attaches_the_provided_workspace() { + let (_tmp, _guard) = with_temp_config_dir(); + let profile = mock_profile("http://127.0.0.1:1"); + let (req, id) = build_request( + &profile, + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + Some("work_abc".to_string()), + false, + None, + vec![], + ) + .unwrap(); + assert_eq!(req.workspace_public_id.as_deref(), Some("work_abc")); + assert_eq!(id.as_deref(), Some("work_abc")); + } + + // --- error message mapping (pure) --------------------------------------------- + + #[test] + fn error_message_maps_not_found() { + let e = SupportError::Http { + status: 404, + body: r#"{"error":"not_found"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("not enabled")); + assert!(msg.contains("support@hotdata.dev")); + } + + #[test] + fn error_message_maps_rate_limited() { + let e = SupportError::Http { + status: 429, + body: r#"{"error":"rate_limited"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("too many reports")); + } + + #[test] + fn error_message_maps_workspace_not_found_with_the_attempted_id() { + let e = SupportError::Http { + status: 404, + body: r#"{"error":"workspace_not_found"}"#.to_string(), + }; + let msg = error_message(&e, Some("work_bad")); + assert!(msg.contains("work_bad"), "got: {msg}"); + assert!(msg.contains("--no-workspace")); + } + + #[test] + fn error_message_maps_401_codes_with_reauth_hint() { + for code in ["missing_authorization", "invalid_api_key"] { + let e = SupportError::Http { + status: 401, + body: format!(r#"{{"error":"{code}"}}"#), + }; + let msg = error_message(&e, None); + assert!(msg.contains("hotdata auth login"), "got: {msg}"); + } + } + + #[test] + fn error_message_maps_body_too_long_with_the_limit() { + let e = SupportError::Http { + status: 422, + body: r#"{"error":"body_too_long"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("20000")); + } + + #[test] + fn error_message_falls_back_to_generic_with_status_and_code() { + let e = SupportError::Http { + status: 403, + body: r#"{"error":"not_a_member"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("403"), "got: {msg}"); + assert!(msg.contains("not_a_member"), "got: {msg}"); + } + + #[test] + fn error_message_falls_back_to_generic_with_no_code() { + let e = SupportError::Http { + status: 502, + body: String::new(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("502"), "got: {msg}"); + } + + // --- report_with_profile: end-to-end against a mock server -------------------- + + #[test] + fn report_happy_path_posts_the_expected_body() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .match_header("Authorization", "Bearer hd_test_key") + .match_body(mockito::Matcher::AllOf(vec![ + mockito::Matcher::PartialJson(serde_json::json!({ + "subject": "CLI hangs on query", + "body": "Every query against work_abc times out after 30s.", + "kind": "bug", + "severity": "high", + "workspace_public_id": "work_abc", + "context": { + "cli_version": env!("CARGO_PKG_VERSION"), + "priority": "urgent", + }, + })), + mockito::Matcher::Regex(r#""idempotency_key":"[0-9a-f]{32}""#.to_string()), + mockito::Matcher::Regex(r#""os":"[a-z]+/[a-z0-9_]+""#.to_string()), + ])) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_happy","status":"queued","subject":"CLI hangs on query","kind":"bug","severity":"high","workspace_public_id":"work_abc","created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + report_with_profile( + &mock_profile(&server.url()), + Some("Every query against work_abc times out after 30s.".to_string()), + Some("CLI hangs on query".to_string()), + "bug".to_string(), + "high".to_string(), + Some("work_abc".to_string()), + false, + None, + vec!["priority=urgent".to_string()], + "table", + ); + + m.assert(); + } + + #[test] + fn report_with_logs_sends_the_redacted_logs_field() { + let (_tmp, _guard) = with_temp_config_dir(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("out.log"); + let raw_logs = "boom\nAuthorization: Bearer topsecrettoken\n"; + std::fs::write(&path, raw_logs).unwrap(); + // What the request must carry: the secret masked, everything else + // untouched. Computed via the function under test rather than + // hand-typed, so this stays in sync with `redact_log_line`'s exact + // masking width. + let expected_logs = redact_logs(raw_logs); + assert!( + !expected_logs.contains("topsecrettoken"), + "test setup bug: expected value still carries the raw secret" + ); + + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .match_body(mockito::Matcher::PartialJson(serde_json::json!({ + "logs": expected_logs, + }))) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_logs","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + report_with_profile( + &mock_profile(&server.url()), + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + None, + true, + Some(path.to_str().unwrap().to_string()), + vec![], + "table", + ); + + // The mock only matches a body carrying the *redacted* logs text, so + // a hit here proves the raw secret never reached the wire. + m.assert(); + } + + #[test] + fn report_200_replay_completes_without_error() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .with_status(200) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_replay","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + report_with_profile( + &mock_profile(&server.url()), + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + None, + true, + None, + vec![], + "json", + ); + + m.assert(); + } +} diff --git a/src/main.rs b/src/main.rs index 064f3ff5..efc890c6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,6 +18,7 @@ use commands::queries::{self, QueriesCommands}; use commands::query::{self, QueryCommands}; use commands::results::{self, ResultsCommands}; use commands::skill::{self, SkillCommands}; +use commands::support::{self, SupportCommands}; use commands::tables; use commands::workspace::{self, WorkspaceCommands}; use commands::{update, usage}; @@ -704,6 +705,29 @@ fn main() { SkillCommands::Status | SkillCommands::List => skill::status(), }, }, + Commands::Support { command } => match command { + SupportCommands::Report { + message, + subject, + kind, + severity, + workspace_id, + no_workspace, + logs, + context, + output, + } => support::report( + message, + subject, + kind, + severity, + workspace_id, + no_workspace, + logs, + context, + &output, + ), + }, }, } } diff --git a/src/util.rs b/src/util.rs index c78b8044..40dfa29e 100644 --- a/src/util.rs +++ b/src/util.rs @@ -24,6 +24,46 @@ pub fn atomic_write(path: &std::path::Path, bytes: &[u8], mode: u32) -> Result<( Ok(()) } +/// Open `$EDITOR` (falling back to `$VISUAL`, then `vi`) on a temp file +/// pre-filled with `initial`, wait for it to exit, and return the file's +/// final contents. `EDITOR`/`VISUAL` may carry arguments (e.g. `code --wait`); +/// the first whitespace-separated token is the program, the rest are passed +/// through before the file path. +/// +/// Not unit-tested: spawning a real editor process has no place in a test +/// suite. Callers that need to test compose behavior should test the pure +/// parsing of the returned text instead. +pub fn open_editor(initial: &str) -> Result { + use std::io::Write; + + let mut tmp = tempfile::Builder::new() + .suffix(".md") + .tempfile() + .map_err(|e| format!("creating temp file: {e}"))?; + tmp.write_all(initial.as_bytes()) + .map_err(|e| format!("writing temp file: {e}"))?; + tmp.flush().map_err(|e| format!("writing temp file: {e}"))?; + let path = tmp.path().to_path_buf(); + + let editor_cmd = std::env::var("EDITOR") + .or_else(|_| std::env::var("VISUAL")) + .unwrap_or_else(|_| "vi".to_string()); + let mut parts = editor_cmd.split_whitespace(); + let program = parts.next().ok_or("EDITOR/VISUAL is set but empty")?; + let args: Vec<&str> = parts.collect(); + + let status = std::process::Command::new(program) + .args(&args) + .arg(&path) + .status() + .map_err(|e| format!("launching editor '{editor_cmd}': {e}"))?; + if !status.success() { + return Err(format!("editor '{editor_cmd}' exited with {status}")); + } + + std::fs::read_to_string(&path).map_err(|e| format!("reading composed file: {e}")) +} + /// Create a steady-ticking spinner with a cyan glyph and the given message. /// Writes to stderr so stdout (json/yaml output) stays clean. pub fn spinner(msg: &str) -> indicatif::ProgressBar { From 235fac8e8328afca8ee11499a86ae7102038254a Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:25:57 -0700 Subject: [PATCH 02/19] docs: document support report command --- README.md | 14 +++++++++++++- skills/hotdata/SKILL.md | 10 +++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fd747f1a..b2770cf7 100644 --- a/README.md +++ b/README.md @@ -129,9 +129,20 @@ Bring your own model with `hotdata search embeddings add`. - `hotdata databases context push|show DATAMODEL` stores your data model as shared, server-side Markdown so humans and agents query with the same map. +## Getting help + +File a support ticket without leaving the terminal: + +```sh +hotdata support report -m "Queries against work_abc have been timing out for an hour" --subject "Queries timing out" +hotdata support report --logs ./stderr.txt --context env=staging +``` + +Omit `-m` in an interactive terminal to compose the report in `$EDITOR` instead. Replies go to the email on your HotData account. + ## Commands -The full command surface. The top level has eight groups — `auth`, `workspaces`, `databases`, `query`, `jobs`, `ingest`, `search`, and `manage`. Run `hotdata --help` for full flags on any command. +The full command surface. The top level has nine groups — `auth`, `workspaces`, `databases`, `query`, `jobs`, `ingest`, `search`, `manage`, and `support`. Run `hotdata --help` for full flags on any command. | Command | What it does | | :-- | :-- | @@ -203,6 +214,7 @@ The full command surface. The top level has eight groups — `auth`, `workspaces | `manage skills install` | Install/update the agent skill into agent directories | | `manage skills status` | Show the agent skill's installation status | | `manage skills list` | List installed skills (alias for `status`) | +| `support report` | File a support ticket with the HotData team | ## Configuration diff --git a/skills/hotdata/SKILL.md b/skills/hotdata/SKILL.md index 361e4597..34005495 100644 --- a/skills/hotdata/SKILL.md +++ b/skills/hotdata/SKILL.md @@ -73,7 +73,7 @@ Catalog, skill decision tree, epic flows (onboard, chain, retrieval), and instan ## Available Commands -Top-level subcommands (each detailed below): **`auth`**, **`query`**, **`workspaces`**, **`databases`**, **`jobs`**, **`ingest`**, **`search`**, **`manage`**. Instant databases nest `databases tables`, `databases queries`, `databases results`, and `databases context`; `ingest` nests `ingest sources`, runs, and logs; `manage` nests `usage`, `completions`, `upgrade`, and `skills`. Search (bm25/vector), indexes, and embedding providers are documented in **`hotdata-search`**; query history, results, Chain, and OLAP patterns in **`hotdata-analytics`**. +Top-level subcommands (each detailed below): **`auth`**, **`query`**, **`workspaces`**, **`databases`**, **`jobs`**, **`ingest`**, **`search`**, **`manage`**, **`support`**. Instant databases nest `databases tables`, `databases queries`, `databases results`, and `databases context`; `ingest` nests `ingest sources`, runs, and logs; `manage` nests `usage`, `completions`, `upgrade`, and `skills`; `support` nests `report`. Search (bm25/vector), indexes, and embedding providers are documented in **`hotdata-search`**; query history, results, Chain, and OLAP patterns in **`hotdata-analytics`**. Global CLI options: **`--api-key`**, **`-v` / `--version`**, **`-h` / `--help`**, **`--no-input`** (disable interactive prompts; commands that require input will error instead — useful in CI or non-TTY environments). Hidden developer flag: **`--debug`** (verbose HTTP logs). @@ -424,6 +424,14 @@ hotdata auth logout # Remove saved auth for the default profile `login` and `register` (both GitHub and `--email`) are **browser-based** PKCE flows: the CLI opens a browser and waits on a local callback to complete sign-in/sign-up — account details (email/password) are entered in the browser, not via CLI flags. They require a browser and an interactive terminal, so they do **not** work under `--no-input` or in headless/CI. For automation, authenticate once interactively, then use the saved session or `HOTDATA_API_KEY`. +### Report a problem (`support report`) + +``` +hotdata support report -m "" --subject "" [--kind bug|question|billing|feature|account|other] [--severity urgent|high|medium|low] [-w | --no-workspace] [--logs |-] [--context KEY=VALUE ...] [-o table|json|yaml] +``` + +Files a support ticket via the API — no browser needed. `-m`/`--subject` are required together for non-interactive use (agents: always pass both); omit both in an interactive terminal to compose in `$EDITOR` instead. Attaches the active workspace by default (`--no-workspace` to omit, `-w` for a specific one); `--logs` reads a file or `-` for stdin (cap 256 KiB); `--context key=value` adds extra diagnostic pairs (repeatable, max 20). Prints the ticket's `public_id` on success — replies go to the email on the HotData account, not to the CLI. + ## Workflows End-to-end recipes — onboard a workspace, run a query, build an instant database (parquet), chain/materialize, add retrieval indexes — live in [references/WORKFLOWS.md](references/WORKFLOWS.md). The command sections above are the per-command reference; the workflows stitch them into sequences. From a70c5096ab494d4da7dfc8d7945ecf93882b4fbb Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:31:27 -0700 Subject: [PATCH 03/19] fix(support): post to api_url, send workspace as a header The support intake is a normal /v1 API-gateway route, not a webapp/ OAuth one: build the URL from api_url via sdk_base_path the same way probe_runtime_status does, and drop the app_url/oauth_base path entirely. Workspace now travels only as X-Workspace-Id (never in the JSON body) so it matches every other authenticated call. --- src/client/jwt.rs | 2 +- src/client/sdk.rs | 2 +- src/client/support.rs | 122 +++++++++++++++++++++++++--------------- src/commands/support.rs | 58 ++++++++++++++----- 4 files changed, 122 insertions(+), 62 deletions(-) diff --git a/src/client/jwt.rs b/src/client/jwt.rs index 04d2b7d8..03f6479f 100644 --- a/src/client/jwt.rs +++ b/src/client/jwt.rs @@ -121,7 +121,7 @@ fn session_from_response( } } -pub(crate) fn oauth_base(profile: &config::ProfileConfig) -> String { +fn oauth_base(profile: &config::ProfileConfig) -> String { // DOT (`/o/authorize/`, `/o/token/`, …) is mounted on the webapp // (app_url), not the API. The api_url host typically only serves // the `/v1` runtimedb routes. diff --git a/src/client/sdk.rs b/src/client/sdk.rs index 1638bc92..0636b6d3 100644 --- a/src/client/sdk.rs +++ b/src/client/sdk.rs @@ -404,7 +404,7 @@ pub fn none_if_404(r: Result) -> Result, ApiError> { /// url through verbatim would produce `/v1/v1/...` on every call. Strip one /// trailing `/v1` (and any trailing slash) so both paths resolve to a single /// `/v1`. -fn sdk_base_path(api_url: &str) -> String { +pub(crate) fn sdk_base_path(api_url: &str) -> String { let trimmed = api_url.trim_end_matches('/'); trimmed.strip_suffix("/v1").unwrap_or(trimmed).to_string() } diff --git a/src/client/support.rs b/src/client/support.rs index 0ca3eedb..e62807f8 100644 --- a/src/client/support.rs +++ b/src/client/support.rs @@ -1,9 +1,9 @@ -//! Raw-HTTP client for `POST {app_url}/v1/support/issues`. +//! Raw-HTTP client for `POST {api_url}/v1/support/issues`. //! -//! Lives on the webapp (`app_url`), not the API gateway (`api_url`) — the -//! same host `jwt::oauth_base` resolves for `/v1/auth/token`. No SDK -//! operation exists for this route yet, so it rides the hand-rolled -//! `reqwest::blocking` seam alongside the token endpoints. +//! This is a normal API-gateway route (`api_url`, default +//! `https://api.hotdata.dev/v1`), not a webapp/OAuth one — same host every +//! other command hits. No SDK operation exists for it yet, so it rides the +//! hand-rolled `reqwest::blocking` seam like `client::ingest`. use crate::client::jwt; use crate::config; @@ -23,8 +23,6 @@ pub struct SupportIssueRequest { pub body: String, pub kind: String, pub severity: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub workspace_public_id: Option, #[serde(skip_serializing_if = "BTreeMap::is_empty")] pub context: BTreeMap, #[serde(skip_serializing_if = "Option::is_none")] @@ -72,25 +70,36 @@ impl SupportError { } } +/// Same construction as `client::sdk`'s `probe_runtime_status`: strip the +/// configured `api_url`'s `/v1` suffix via `sdk_base_path`, then add the one +/// `/v1/support/issues` itself expects. fn url(profile: &config::ProfileConfig) -> String { - format!("{}/v1/support/issues", jwt::oauth_base(profile)) + let base = crate::client::sdk::sdk_base_path(&profile.api_url); + format!("{}/v1/support/issues", base.trim_end_matches('/')) } fn send_once( client: &reqwest::blocking::Client, profile: &config::ProfileConfig, token: &str, + workspace_id: Option<&str>, req: &SupportIssueRequest, ) -> Result<(SupportIssue, bool), SupportError> { let body = serde_json::to_value(req).expect("SupportIssueRequest serializes"); - let builder = client + let mut builder = client .post(url(profile)) .header("Authorization", format!("Bearer {token}")) .header( "User-Agent", concat!("hotdata-cli/", env!("CARGO_PKG_VERSION")), - ) - .json(&body); + ); + // Same header every other /v1 call carries: the gateway ranks a longer + // path prefix above a header match, so /v1/support keeps routing here + // (not to a workspace's runtimedb worker) even with it present. + if let Some(ws) = workspace_id { + builder = builder.header("X-Workspace-Id", ws); + } + let builder = builder.json(&body); let (status, body_text) = util::send_debug(client, builder, Some(&body)) .map_err(|e| SupportError::Connection(e.to_string()))?; if !status.is_success() { @@ -107,18 +116,21 @@ fn send_once( Ok((parsed.issue, replay)) } -/// File a support issue. Retries once, after [`RETRY_DELAY`], on a connection -/// error or 5xx — never on a 4xx — reusing the same `idempotency_key` so a -/// retried create can't double-file. +/// File a support issue. `workspace_id`, when given, is sent only as the +/// `X-Workspace-Id` header (never in the JSON body). Retries once, after +/// [`RETRY_DELAY`], on a connection error or 5xx — never on a 4xx — reusing +/// the same `idempotency_key` so a retried create can't double-file. pub fn post_support_issue( profile: &config::ProfileConfig, + workspace_id: Option<&str>, req: &SupportIssueRequest, ) -> Result<(SupportIssue, bool), SupportError> { - post_support_issue_with_delay(profile, req, RETRY_DELAY) + post_support_issue_with_delay(profile, workspace_id, req, RETRY_DELAY) } fn post_support_issue_with_delay( profile: &config::ProfileConfig, + workspace_id: Option<&str>, req: &SupportIssueRequest, retry_delay: Duration, ) -> Result<(SupportIssue, bool), SupportError> { @@ -131,11 +143,11 @@ fn post_support_issue_with_delay( let token = jwt::ensure_access_token(profile, api_key_fallback).map_err(SupportError::Auth)?; let client = crate::client::raw_http::build_http_client(); - match send_once(&client, profile, &token, req) { + match send_once(&client, profile, &token, workspace_id, req) { Ok(ok) => Ok(ok), Err(e) if e.is_retryable() => { std::thread::sleep(retry_delay); - send_once(&client, profile, &token, req) + send_once(&client, profile, &token, workspace_id, req) } Err(e) => Err(e), } @@ -144,14 +156,13 @@ fn post_support_issue_with_delay( #[cfg(test)] mod tests { use super::*; - use crate::config::{ApiUrl, AppUrl, ProfileConfig, test_helpers::with_temp_config_dir}; + use crate::config::{ApiUrl, ProfileConfig, test_helpers::with_temp_config_dir}; /// A profile with an api_key set, so every test here resolves a bearer /// with zero network calls (no session mint/refresh to mock separately). fn mock_profile(url: &str) -> ProfileConfig { ProfileConfig { api_key: Some("hd_test_key".to_string()), - app_url: AppUrl(Some(url.to_string())), api_url: ApiUrl(Some(url.to_string())), ..Default::default() } @@ -163,7 +174,6 @@ mod tests { body: "Queries against my workspace have been hanging for an hour.".into(), kind: "bug".into(), severity: "high".into(), - workspace_public_id: Some("work_abc".into()), context: BTreeMap::from([("cli_version".to_string(), "0.31.0".to_string())]), logs: None, idempotency_key: idempotency_key.to_string(), @@ -178,11 +188,11 @@ mod tests { .mock("POST", "/v1/support/issues") .match_header("Authorization", "Bearer hd_test_key") .match_header("content-type", "application/json") + .match_header("X-Workspace-Id", "work_abc") .match_body(mockito::Matcher::PartialJson(serde_json::json!({ "subject": "Query timing out", "kind": "bug", "severity": "high", - "workspace_public_id": "work_abc", "idempotency_key": "fixed-key-1", }))) .with_status(202) @@ -193,15 +203,41 @@ mod tests { .create(); let profile = mock_profile(&server.url()); - let mut r = req("fixed-key-1"); - r.workspace_public_id = Some("work_abc".into()); - let (issue, replay) = post_support_issue_with_delay(&profile, &r, Duration::ZERO).unwrap(); + let (issue, replay) = post_support_issue_with_delay( + &profile, + Some("work_abc"), + &req("fixed-key-1"), + Duration::ZERO, + ) + .unwrap(); m.assert(); assert_eq!(issue.public_id, "supp_1"); assert_eq!(issue.status, "queued"); assert!(!replay); } + #[test] + fn no_workspace_sends_no_x_workspace_id_header() { + // The JSON body never carries a workspace field at all — see + // `SupportIssueRequest`, which has no such field to omit — so the + // only thing left to assert is the header. + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .match_header("X-Workspace-Id", mockito::Matcher::Missing) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_none","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + let profile = mock_profile(&server.url()); + post_support_issue_with_delay(&profile, None, &req("k"), Duration::ZERO).unwrap(); + m.assert(); + } + #[test] fn replay_200_is_reported_as_replay() { let (_tmp, _guard) = with_temp_config_dir(); @@ -217,7 +253,8 @@ mod tests { let profile = mock_profile(&server.url()); let (issue, replay) = - post_support_issue_with_delay(&profile, &req("fixed-key-2"), Duration::ZERO).unwrap(); + post_support_issue_with_delay(&profile, None, &req("fixed-key-2"), Duration::ZERO) + .unwrap(); m.assert(); assert_eq!(issue.public_id, "supp_2"); assert!(replay); @@ -251,7 +288,8 @@ mod tests { let profile = mock_profile(&server.url()); let (issue, replay) = - post_support_issue_with_delay(&profile, &req("fixed-key-3"), Duration::ZERO).unwrap(); + post_support_issue_with_delay(&profile, None, &req("fixed-key-3"), Duration::ZERO) + .unwrap(); fail.assert(); ok.assert(); assert_eq!(issue.public_id, "supp_3"); @@ -264,7 +302,8 @@ mod tests { // transport level, and the caller sees a Connection error, not a hang. let (_tmp, _guard) = with_temp_config_dir(); let profile = mock_profile("http://127.0.0.1:1"); - let err = post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap_err(); + let err = + post_support_issue_with_delay(&profile, None, &req("k"), Duration::ZERO).unwrap_err(); assert!(matches!(err, SupportError::Connection(_))); } @@ -281,29 +320,20 @@ mod tests { .create(); let profile = mock_profile(&server.url()); - let err = post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap_err(); + let err = + post_support_issue_with_delay(&profile, None, &req("k"), Duration::ZERO).unwrap_err(); m.assert(); assert!(matches!(err, SupportError::Http { status: 422, .. })); } #[test] - fn uses_app_url_not_api_url() { - // app_url and api_url point at different hosts; the request must hit - // the webapp host (app_url), never the api_url one. - let (_tmp, _guard) = with_temp_config_dir(); - let mut server = mockito::Server::new(); - let m = server - .mock("POST", "/v1/support/issues") - .with_status(202) - .with_header("content-type", "application/json") - .with_body( - r#"{"ok":true,"issue":{"public_id":"supp_4","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, - ) - .create(); - - let mut profile = mock_profile(&server.url()); - profile.api_url = ApiUrl(Some("http://127.0.0.1:1".to_string())); - post_support_issue_with_delay(&profile, &req("k"), Duration::ZERO).unwrap(); - m.assert(); + fn url_strips_the_configured_v1_suffix_and_adds_it_back_once() { + // DEFAULT_API_URL carries a /v1 suffix; sdk_base_path strips it so + // this doesn't add up to /v1/v1/support/issues. + let profile = ProfileConfig { + api_url: ApiUrl(Some("https://api.hotdata.dev/v1".to_string())), + ..Default::default() + }; + assert_eq!(url(&profile), "https://api.hotdata.dev/v1/support/issues"); } } diff --git a/src/commands/support.rs b/src/commands/support.rs index 80d2863e..67cf3676 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -1,5 +1,5 @@ -//! `hotdata support report` — file a ticket through the webapp's support -//! intake (`POST {app_url}/v1/support/issues`). `client::support` owns the +//! `hotdata support report` — file a ticket through the API's support +//! intake (`POST {api_url}/v1/support/issues`). `client::support` owns the //! HTTP call; this module owns composing the request and rendering the //! result. //! @@ -130,7 +130,7 @@ fn report_with_profile( std::process::exit(1); }); - match client::support::post_support_issue(profile, &req) { + match client::support::post_support_issue(profile, workspace_id.as_deref(), &req) { Ok((issue, replay)) => print_success(&issue, replay, output), Err(e) => handle_error(&e, workspace_id.as_deref()), } @@ -169,7 +169,6 @@ fn build_request( body, kind, severity, - workspace_public_id: workspace_id.clone(), context, logs, idempotency_key: generate_idempotency_key(), @@ -480,12 +479,11 @@ fn handle_error(e: &SupportError, workspace_id: Option<&str>) -> ! { #[cfg(test)] mod tests { use super::*; - use crate::config::{ApiUrl, AppUrl, ProfileConfig, test_helpers::with_temp_config_dir}; + use crate::config::{ApiUrl, ProfileConfig, test_helpers::with_temp_config_dir}; fn mock_profile(url: &str) -> ProfileConfig { ProfileConfig { api_key: Some("hd_test_key".to_string()), - app_url: AppUrl(Some(url.to_string())), api_url: ApiUrl(Some(url.to_string())), ..Default::default() } @@ -790,10 +788,14 @@ Second paragraph. } #[test] - fn build_request_no_workspace_omits_workspace_public_id() { + fn build_request_no_workspace_resolves_to_none() { + // The request itself never carries a workspace field (see + // `SupportIssueRequest` — no such field exists to omit); the resolved + // id travels alongside the request instead, for the `X-Workspace-Id` + // header and for a `workspace_not_found` error message. let (_tmp, _guard) = with_temp_config_dir(); let profile = mock_profile("http://127.0.0.1:1"); - let (req, id) = build_request( + let (_req, id) = build_request( &profile, Some("body".to_string()), Some("Subj".to_string()), @@ -805,15 +807,14 @@ Second paragraph. vec![], ) .unwrap(); - assert_eq!(req.workspace_public_id, None); assert_eq!(id, None); } #[test] - fn build_request_attaches_the_provided_workspace() { + fn build_request_resolves_the_provided_workspace() { let (_tmp, _guard) = with_temp_config_dir(); let profile = mock_profile("http://127.0.0.1:1"); - let (req, id) = build_request( + let (_req, id) = build_request( &profile, Some("body".to_string()), Some("Subj".to_string()), @@ -825,7 +826,6 @@ Second paragraph. vec![], ) .unwrap(); - assert_eq!(req.workspace_public_id.as_deref(), Some("work_abc")); assert_eq!(id.as_deref(), Some("work_abc")); } @@ -909,19 +909,19 @@ Second paragraph. // --- report_with_profile: end-to-end against a mock server -------------------- #[test] - fn report_happy_path_posts_the_expected_body() { + fn report_happy_path_posts_the_expected_body_and_workspace_header() { let (_tmp, _guard) = with_temp_config_dir(); let mut server = mockito::Server::new(); let m = server .mock("POST", "/v1/support/issues") .match_header("Authorization", "Bearer hd_test_key") + .match_header("X-Workspace-Id", "work_abc") .match_body(mockito::Matcher::AllOf(vec![ mockito::Matcher::PartialJson(serde_json::json!({ "subject": "CLI hangs on query", "body": "Every query against work_abc times out after 30s.", "kind": "bug", "severity": "high", - "workspace_public_id": "work_abc", "context": { "cli_version": env!("CARGO_PKG_VERSION"), "priority": "urgent", @@ -953,6 +953,36 @@ Second paragraph. m.assert(); } + #[test] + fn report_no_workspace_sends_no_workspace_header() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .match_header("X-Workspace-Id", mockito::Matcher::Missing) + .with_status(202) + .with_header("content-type", "application/json") + .with_body( + r#"{"ok":true,"issue":{"public_id":"supp_none","status":"queued","subject":"s","kind":"bug","severity":"high","workspace_public_id":null,"created_at":"2026-09-05T00:00:00Z"}}"#, + ) + .create(); + + report_with_profile( + &mock_profile(&server.url()), + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + Some("work_ignored".to_string()), + true, + None, + vec![], + "table", + ); + + m.assert(); + } + #[test] fn report_with_logs_sends_the_redacted_logs_field() { let (_tmp, _guard) = with_temp_config_dir(); From 2a148575533b9de1983377964860cb31c1d96b67 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:32:38 -0700 Subject: [PATCH 04/19] style(support): drop trailing period from fallback error message Match the repo's prevailing convention for "error: ..." lines (see client::ingest's IngestError::message) rather than the literal punctuation in the draft spec text. --- src/commands/support.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 67cf3676..f0a9e326 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -455,8 +455,8 @@ fn error_message(e: &SupportError, workspace_id: Option<&str>) -> String { Some("body_too_long") => { "report body is too long (limit 20000 characters)".to_string() } - Some(code) => format!("support request failed ({status} {code})."), - None => format!("support request failed ({status})."), + Some(code) => format!("support request failed ({status} {code})"), + None => format!("support request failed ({status})"), } } } From 4009d9549e08669ebbccc39ec4873d68ccd356d0 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:34:40 -0700 Subject: [PATCH 05/19] fix(support): reject over-long subjects client-side Validate the subject after compose (both -m/--subject and $EDITOR paths): more than 200 chars errors out before any HTTP call, matching the server's own subject_too_long limit. Map subject_too_long, subject_required, and body_required in error_message instead of letting them fall through to the generic status/code message. --- src/commands/support.rs | 76 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/commands/support.rs b/src/commands/support.rs index f0a9e326..fd8395bd 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -18,6 +18,7 @@ use std::collections::BTreeMap; const MAX_LOGS_BYTES: usize = 256 * 1024; const MAX_CONTEXT_VALUE_CHARS: usize = 500; const MAX_USER_CONTEXT_PAIRS: usize = 20; +const MAX_SUBJECT_CHARS: usize = 200; /// The one non-error exit message ("aborted, nothing sent") shares the error /// channel (`Result<_, String>`) with real validation failures; this marker /// is how the top-level caller tells them apart to skip the "error: " prefix. @@ -155,6 +156,7 @@ fn build_request( let (workspace_id, workspace_locked) = resolve_optional_workspace(profile, workspace_id, no_workspace)?; let (subject, body) = compose(message, subject)?; + validate_subject(&subject)?; let mut context = default_context(profile, workspace_locked); merge_user_context(&mut context, &context_pairs)?; @@ -261,6 +263,19 @@ fn truncate_chars(s: &str, max: usize) -> String { s.chars().take(max).collect() } +/// Reject an over-long subject before any HTTP call — the server enforces +/// the same 200-char limit (`subject_too_long`), but there is no reason to +/// round-trip a request we already know it will refuse. +fn validate_subject(subject: &str) -> Result<(), String> { + let len = subject.chars().count(); + if len > MAX_SUBJECT_CHARS { + return Err(format!( + "subject is too long ({len} chars; limit {MAX_SUBJECT_CHARS})" + )); + } + Ok(()) +} + fn default_context( profile: &config::ProfileConfig, workspace_locked: bool, @@ -455,6 +470,11 @@ fn error_message(e: &SupportError, workspace_id: Option<&str>) -> String { Some("body_too_long") => { "report body is too long (limit 20000 characters)".to_string() } + Some("subject_too_long") => { + "subject is too long (limit 200 characters)".to_string() + } + Some("subject_required") => "subject is required".to_string(), + Some("body_required") => "report body is required".to_string(), Some(code) => format!("support request failed ({status} {code})"), None => format!("support request failed ({status})"), } @@ -737,6 +757,30 @@ Second paragraph. m.assert(); } + #[test] + fn build_request_over_long_subject_errors_before_any_http_call() { + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server.mock("POST", "/v1/support/issues").expect(0).create(); + + let subject = "x".repeat(MAX_SUBJECT_CHARS + 1); + let err = build_request( + &mock_profile(&server.url()), + Some("body".to_string()), + Some(subject), + "bug".to_string(), + "high".to_string(), + None, + true, + None, + vec![], + ) + .unwrap_err(); + assert!(err.contains("201"), "got: {err}"); + assert!(err.contains("limit 200"), "got: {err}"); + m.assert(); + } + #[test] fn build_request_logs_over_cap_errors_before_any_http_call() { let (_tmp, _guard) = with_temp_config_dir(); @@ -885,6 +929,38 @@ Second paragraph. assert!(msg.contains("20000")); } + #[test] + fn error_message_maps_subject_too_long_with_the_limit() { + let e = SupportError::Http { + status: 422, + body: r#"{"error":"subject_too_long"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("200"), "got: {msg}"); + } + + #[test] + fn error_message_maps_subject_required() { + let e = SupportError::Http { + status: 422, + body: r#"{"error":"subject_required"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("subject"), "got: {msg}"); + assert!(msg.contains("required"), "got: {msg}"); + } + + #[test] + fn error_message_maps_body_required() { + let e = SupportError::Http { + status: 422, + body: r#"{"error":"body_required"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("body"), "got: {msg}"); + assert!(msg.contains("required"), "got: {msg}"); + } + #[test] fn error_message_falls_back_to_generic_with_status_and_code() { let e = SupportError::Http { From 113bc665a38168e3eacc829eda7aaf7f42b0377e Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:55:55 -0700 Subject: [PATCH 06/19] fix(util): mask_credential no longer panics on non-ASCII Byte-slicing a string at fixed offsets panics the moment a boundary lands mid multi-byte character. Slice by char instead; ASCII behavior (head+tail, head-only, or "***") is unchanged. --- src/util.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/util.rs b/src/util.rs index 40dfa29e..a79b0b64 100644 --- a/src/util.rs +++ b/src/util.rs @@ -173,13 +173,22 @@ pub fn debug_response_redacted( /// (`XXXX...YYYY`), or `***` if it's too short to reveal anything /// safely. The tail makes it easy to distinguish which token is on /// the wire (e.g. user JWT vs database-scoped JWT vs opaque API token). +/// +/// Counts and slices by `char`, not byte: real credentials are ASCII, but +/// this also runs over arbitrary `--logs` text, and byte-slicing an +/// arbitrary string panics the moment it lands mid multi-byte character. pub fn mask_credential(s: &str) -> String { - if s.len() >= 12 { - format!("{}...{}", &s[..4], &s[s.len() - 4..]) - } else if s.len() > 4 { + let chars: Vec = s.chars().collect(); + let len = chars.len(); + if len >= 12 { + let head: String = chars[..4].iter().collect(); + let tail: String = chars[len - 4..].iter().collect(); + format!("{head}...{tail}") + } else if len > 4 { // Short-ish — still better to show head than nothing, but - // don't double up on bytes by showing a tail. - format!("{}...", &s[..4]) + // don't double up on chars by showing a tail. + let head: String = chars[..4].iter().collect(); + format!("{head}...") } else { "***".into() } @@ -558,6 +567,13 @@ mod tests { assert_eq!(mask_credential(""), "***"); } + #[test] + fn mask_credential_non_ascii_does_not_panic() { + // Byte-slicing this would panic mid multi-byte char; char-slicing + // must not. 14 chars total, so the long-form head+tail branch. + assert_eq!(mask_credential("token€12345678"), "toke...5678"); + } + #[test] fn api_error_humanizes_snake_case_code() { // Django-style flat shape — `workspace_not_found` should render From d6e8197cb12e78b1df325fb02db46ed843adf026 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:57:07 -0700 Subject: [PATCH 07/19] fix(support): redact a Bearer token anywhere on a log line redact_log_line only masked an Authorization: header at the start of a line, missing a pasted `curl -H "Authorization: Bearer ..."` or a timestamp-prefixed access-log line. Scan for "bearer " case- insensitively at any position and mask the token that follows; fall back to the plain Authorization: header case (no Bearer scheme) only when no Bearer token was found, so nothing is masked twice. --- src/commands/support.rs | 94 +++++++++++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index fd8395bd..34b23034 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -362,21 +362,63 @@ fn redact_logs(text: &str) -> String { .join("\n") } +/// A saved log can carry a credential anywhere on the line, not just after a +/// header name at the start (a pasted `curl -H "Authorization: Bearer ..."`, +/// a timestamp-prefixed access log). Mask every `bearer ` found +/// case-insensitively at any position; fall back to the plain +/// `Authorization:` header case (no `Bearer` scheme) only when no token was +/// found that way, so a Bearer-scheme value is never masked twice. fn redact_log_line(line: &str) -> String { - let trimmed = line.trim_start(); - let indent = &line[..line.len() - trimmed.len()]; - let Some((name, value)) = trimmed.split_once(':') else { - return line.to_string(); - }; - if !name.eq_ignore_ascii_case("authorization") { - return line.to_string(); + if let Some(masked) = mask_bearer_tokens(line) { + return masked; } - let value = value.trim(); - let masked = match value.strip_prefix("Bearer ") { - Some(token) => format!("Bearer {}", util::mask_credential(token)), - None => util::mask_credential(value), - }; - format!("{indent}Authorization: {masked}") + mask_authorization_header_value(line).unwrap_or_else(|| line.to_string()) +} + +/// Find every case-insensitive `bearer ` in `line` and mask the token that +/// follows it — the run of chars up to whitespace, a quote, or end of line — +/// keeping "Bearer" (in whatever case it was written) and everything else on +/// the line untouched. `None` when the line has no `bearer ` at all. +fn mask_bearer_tokens(line: &str) -> Option { + // `to_ascii_lowercase` only rewrites ASCII bytes in place, so `lower` and + // `line` share byte offsets even over multi-byte UTF-8 text — safe to + // search one and slice the other. + let lower = line.to_ascii_lowercase(); + let mut out = String::with_capacity(line.len()); + let mut pos = 0usize; + let mut found_any = false; + while let Some(rel) = lower[pos..].find("bearer ") { + found_any = true; + let keep_end = pos + rel + "bearer ".len(); + out.push_str(&line[pos..keep_end]); + let token_start = keep_end; + let token_len = line[token_start..] + .find(|c: char| c.is_whitespace() || c == '"' || c == '\'') + .unwrap_or(line.len() - token_start); + out.push_str(&util::mask_credential( + &line[token_start..token_start + token_len], + )); + pos = token_start + token_len; + } + if !found_any { + return None; + } + out.push_str(&line[pos..]); + Some(out) +} + +/// A bare `Authorization: ` with no `Bearer` scheme (e.g. a raw +/// `hd_...` token) — mask the whole value, keeping the header name +/// canonically capitalized (matching prior behavior) and everything before +/// it on the line untouched. `authorization:` is located anywhere in the +/// line, not just at its start. +fn mask_authorization_header_value(line: &str) -> Option { + let lower = line.to_ascii_lowercase(); + let idx = lower.find("authorization:")?; + let indent = &line[..idx]; + let value_start = idx + "authorization:".len(); + let masked = util::mask_credential(line[value_start..].trim()); + Some(format!("{indent}Authorization: {masked}")) } fn generate_idempotency_key() -> String { @@ -665,6 +707,32 @@ Second paragraph. assert_eq!(out, " Authorization: hd_a...ijkl"); } + #[test] + fn redact_logs_masks_a_curl_dash_h_bearer_token_mid_line() { + let input = r#"curl -H "Authorization: Bearer hd_live_x123456789" https://api.hotdata.dev/v1/query"#; + let out = redact_logs(input); + assert!( + out.contains(r#"Authorization: Bearer hd_l...6789""#), + "got: {out}" + ); + assert!(out.contains("https://api.hotdata.dev/v1/query")); + assert!(!out.contains("hd_live_x123456789")); + } + + #[test] + fn redact_logs_masks_a_timestamp_prefixed_bearer_line() { + let input = "2026-09-05T10:00:00Z Authorization: Bearer supersecrettoken1234"; + let out = redact_logs(input); + assert!(out.starts_with("2026-09-05T10:00:00Z Authorization: Bearer ")); + assert!(!out.contains("supersecrettoken1234")); + } + + #[test] + fn redact_logs_line_with_no_credential_is_unchanged() { + let input = "GET /v1/foo 200 12ms"; + assert_eq!(redact_logs(input), input); + } + // --- idempotency key -------------------------------------------------------- #[test] From 78a0bd83e9787acbe18d616eefe4db2dada5a989 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:58:08 -0700 Subject: [PATCH 08/19] fix(support): surface server text on an uncoded error status error_message's generic fallback for a status with no stable error code (an upstream 5xx, a framework-level rejection) printed only the bare status, dropping whatever the body said (e.g. a FastAPI {"detail": ...}). Fold in util::api_error's rendering, truncated so an unbounded non-JSON body can't flood the terminal. --- src/commands/support.rs | 43 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 34b23034..f7a54926 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -19,6 +19,10 @@ const MAX_LOGS_BYTES: usize = 256 * 1024; const MAX_CONTEXT_VALUE_CHARS: usize = 500; const MAX_USER_CONTEXT_PAIRS: usize = 20; const MAX_SUBJECT_CHARS: usize = 200; +/// Cap on the server text folded into a generic error-message fallback — an +/// HTML page is already short-circuited by `util::api_error`, but a plain +/// non-JSON body (a raw stack trace, say) is echoed back verbatim. +const MAX_ERROR_BODY_CHARS: usize = 200; /// The one non-error exit message ("aborted, nothing sent") shares the error /// channel (`Result<_, String>`) with real validation failures; this marker /// is how the top-level caller tells them apart to skip the "error: " prefix. @@ -518,7 +522,15 @@ fn error_message(e: &SupportError, workspace_id: Option<&str>) -> String { Some("subject_required") => "subject is required".to_string(), Some("body_required") => "report body is required".to_string(), Some(code) => format!("support request failed ({status} {code})"), - None => format!("support request failed ({status})"), + // No stable code at all (an upstream 5xx, a framework-level + // rejection) — still surface whatever the server said rather + // than a bare status. `api_error` echoes a non-JSON, + // non-HTML body verbatim, which could be an unbounded + // stack trace; truncate so that can't flood the terminal. + None => format!( + "support request failed ({status}): {}", + truncate_chars(&util::api_error(body.clone()), MAX_ERROR_BODY_CHARS) + ), } } } @@ -1050,6 +1062,35 @@ Second paragraph. assert!(msg.contains("502"), "got: {msg}"); } + #[test] + fn error_message_no_code_includes_the_server_text() { + // No stable `error.code`/flat code at all, but the body still says + // something useful (a FastAPI-style `detail`) — don't drop it. + let e = SupportError::Http { + status: 502, + body: r#"{"detail":"upstream timeout"}"#.to_string(), + }; + let msg = error_message(&e, None); + assert!(msg.contains("502"), "got: {msg}"); + assert!(msg.contains("upstream timeout"), "got: {msg}"); + } + + #[test] + fn error_message_no_code_truncates_a_long_raw_body() { + // A non-JSON, non-HTML body is echoed verbatim by `util::api_error` + // — an unbounded stack trace must not flood the terminal. + let e = SupportError::Http { + status: 500, + body: "x".repeat(MAX_ERROR_BODY_CHARS + 500), + }; + let msg = error_message(&e, None); + assert!( + msg.chars().count() < MAX_ERROR_BODY_CHARS + 100, + "message not truncated, got {} chars", + msg.chars().count() + ); + } + // --- report_with_profile: end-to-end against a mock server -------------------- #[test] From 2ad0cd01c49413bc9ddbe8d2683cefa219a7c4de Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 10:59:38 -0700 Subject: [PATCH 09/19] fix(support): never probe the network to resolve a workspace default resolve_optional_workspace called client::credentials::default_workspace_id, which issues a live GET /workspaces for an env/flag-sourced api key. A support report must not block on the API being slow or down -- that's exactly when this command runs. Read only the saved default workspace from the profile instead; an exact workspace is optional for filing. --- src/commands/support.rs | 54 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index f7a54926..73501c3d 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -211,7 +211,19 @@ fn resolve_optional_workspace( if let Some(id) = provided { return Ok((Some(id), false)); } - Ok((client::credentials::default_workspace_id(profile), false)) + // Deliberately NOT `client::credentials::default_workspace_id`: for an + // api-key credential (`--api-key`/`HOTDATA_API_KEY`) that helper probes + // `GET /workspaces` to discover scope. An exact workspace is optional for + // filing a report, and the API being slow or down is exactly the + // situation this command exists for — it must never block on a network + // round trip just to guess a default. Read only the saved default + // (`workspaces set` / a prior login moves one to the front); if there is + // none, or the current credential can't actually reach it, file with no + // workspace instead of guessing. + Ok(( + profile.workspaces.first().map(|w| w.public_id.clone()), + false, + )) } /// Produce (subject, body) from `-m`/`--subject`, or by composing in @@ -797,6 +809,40 @@ Second paragraph. assert!(!locked); } + #[test] + fn build_request_with_env_api_key_and_no_configured_default_makes_zero_http_calls() { + // `client::credentials::default_workspace_id` would probe `GET + // /workspaces` for an env/flag-sourced api key with no single- + // workspace answer already known -- resolve_optional_workspace must + // never do that. A report is exactly what gets filed when the API + // is slow or down, so filing one must never block on it. + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let probe = server.mock("GET", "/workspaces").expect(0).create(); + + let mut profile = mock_profile(&server.url()); + profile.api_key_source = config::ApiKeySource::Env; + assert!( + profile.workspaces.is_empty(), + "test setup: no saved default" + ); + + let (_req, id) = build_request( + &profile, + Some("body".to_string()), + Some("Subj".to_string()), + "bug".to_string(), + "high".to_string(), + None, + false, + None, + vec![], + ) + .unwrap(); + assert_eq!(id, None); + probe.assert(); + } + #[test] fn saved_default_is_used_when_no_flag_given() { let profile = ProfileConfig { @@ -832,8 +878,10 @@ Second paragraph. ) .unwrap_err(); assert!(err.contains("KEY=VALUE"), "got: {err}"); - // build_request never talks to the network at all, so this always - // holds — asserted anyway as the documented guarantee. + // build_request never talks to the network at all -- workspace + // resolution reads only the saved default, never probes -- so this + // always holds regardless of which validation failed; asserted + // anyway as the documented guarantee. m.assert(); } From ddef38f7918d683bb8cadbe9d211fd216803c127 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 11:02:17 -0700 Subject: [PATCH 10/19] fix(support): persist an editor-composed report on a failed send open_editor's temp file is gone by the time a POST fails, so a lost send used to lose the text the user just wrote with no way to recover it. compose() now reports whether the text came from $EDITOR; send_and_report persists it to support-draft-.md (mode 0600) under the config dir on any failed send when it did, and prints a re-file hint (or the whole report, if even saving fails). The -m/--subject path needs nothing extra -- that text is still in shell history. --- src/commands/support.rs | 260 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 242 insertions(+), 18 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 73501c3d..053f6025 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -14,6 +14,8 @@ use crate::config; use crate::util; use serde::Serialize; use std::collections::BTreeMap; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; const MAX_LOGS_BYTES: usize = 256 * 1024; const MAX_CONTEXT_VALUE_CHARS: usize = 500; @@ -115,7 +117,7 @@ fn report_with_profile( context_pairs: Vec, output: &str, ) { - let (req, workspace_id) = build_request( + let (req, workspace_id, from_editor) = build_request( profile, message, subject, @@ -135,16 +137,87 @@ fn report_with_profile( std::process::exit(1); }); - match client::support::post_support_issue(profile, workspace_id.as_deref(), &req) { + send_and_report(profile, req, workspace_id, from_editor, output); +} + +/// Send the built request and render the result. Split out from +/// `report_with_profile` so a test can drive it directly with a hand-built +/// request and an explicit `from_editor`, without spawning `$EDITOR`. +fn send_and_report( + profile: &config::ProfileConfig, + req: SupportIssueRequest, + workspace_id: Option, + from_editor: bool, + output: &str, +) { + let result = client::support::post_support_issue(profile, workspace_id.as_deref(), &req); + persist_on_editor_failure(&result, &req, from_editor); + match result { Ok((issue, replay)) => print_success(&issue, replay, output), Err(e) => handle_error(&e, workspace_id.as_deref()), } } +/// If the report was composed in `$EDITOR` and the send failed, persist it — +/// `open_editor`'s own temp file is already gone by now, so a lost send +/// would otherwise lose the text the user just wrote. A no-op for the +/// `-m`/`--subject` path (that text is still in the caller's shell history) +/// and for a successful send. Never touches `result`; it only adds a side +/// effect alongside it. +fn persist_on_editor_failure( + result: &Result<(SupportIssue, bool), SupportError>, + req: &SupportIssueRequest, + from_editor: bool, +) { + if from_editor && result.is_err() { + persist_composed_report(&req.subject, &req.body); + } +} + +/// Save the just-composed report and tell the user how to re-file it, or — +/// if even that fails — print the whole thing to stderr so nothing is lost. +fn persist_composed_report(subject: &str, body: &str) { + match save_draft(subject, body) { + Ok(path) => { + let path = path.display(); + eprintln!( + "Your report was saved to {path}. Re-file it with: hotdata support report --subject '{subject}' -m \"$(tail -n +3 {path})\"" + ); + } + Err(e) => { + eprintln!("warning: could not save your report to disk: {e}"); + eprintln!("--- your report, so nothing is lost ---"); + eprintln!("Subject: {subject}"); + eprintln!(); + eprintln!("{body}"); + } + } +} + +/// Persist a composed report to disk as `support-draft-.md` +/// under the CLI config dir (mode 0600, same as the session file — the +/// content is the user's own report, not a credential, but there is no +/// reason to make it more visible than that). Format is `"\n\n +/// \n"`, so `tail -n +3 ` recovers the body alone (matching the +/// re-file hint in [`persist_composed_report`]). +fn save_draft(subject: &str, body: &str) -> Result { + let dir = config::config_dir()?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let path = dir.join(format!("support-draft-{now}.md")); + let content = format!("{subject}\n\n{body}\n"); + util::atomic_write(&path, content.as_bytes(), 0o600)?; + Ok(path) +} + /// Resolve the workspace, compose the report text, build context, and load /// logs — everything that can be rejected before an HTTP call is ever made. -/// Returns the request plus the resolved workspace id (needed to render a -/// `workspace_not_found` error message later). +/// Returns the request, the resolved workspace id (needed to render a +/// `workspace_not_found` error message later), and whether the text came +/// from `$EDITOR` (needed to decide whether a failed send should be +/// persisted to disk). #[allow(clippy::too_many_arguments)] fn build_request( profile: &config::ProfileConfig, @@ -156,10 +229,10 @@ fn build_request( no_workspace: bool, logs_path: Option, context_pairs: Vec, -) -> Result<(SupportIssueRequest, Option), String> { +) -> Result<(SupportIssueRequest, Option, bool), String> { let (workspace_id, workspace_locked) = resolve_optional_workspace(profile, workspace_id, no_workspace)?; - let (subject, body) = compose(message, subject)?; + let (subject, body, from_editor) = compose(message, subject)?; validate_subject(&subject)?; let mut context = default_context(profile, workspace_locked); @@ -179,7 +252,7 @@ fn build_request( logs, idempotency_key: generate_idempotency_key(), }; - Ok((req, workspace_id)) + Ok((req, workspace_id, from_editor)) } /// Resolve the workspace to attach. Unlike `main::resolve_workspace`, an @@ -226,16 +299,22 @@ fn resolve_optional_workspace( )) } -/// Produce (subject, body) from `-m`/`--subject`, or by composing in -/// `$EDITOR` when neither is usable. The abort case (empty compose) is -/// signaled via the literal [`ABORTED`] string so the caller skips the -/// "error: " prefix on it. -fn compose(message: Option, subject: Option) -> Result<(String, String), String> { +/// Produce (subject, body, from_editor) from `-m`/`--subject`, or by +/// composing in `$EDITOR` when neither is usable. `from_editor` is what lets +/// a failed send later decide whether to persist a draft: `-m` text is still +/// in the caller's shell history, but `$EDITOR`'s own temp file is gone by +/// the time a send fails, so that text has nowhere else to live. The abort +/// case (empty compose) is signaled via the literal [`ABORTED`] string so the +/// caller skips the "error: " prefix on it. +fn compose( + message: Option, + subject: Option, +) -> Result<(String, String, bool), String> { if let Some(body) = message { let Some(subject) = subject else { return Err("--subject is required when using -m/--message".to_string()); }; - return Ok((subject, body)); + return Ok((subject, body, false)); } if !util::is_interactive() { @@ -252,7 +331,8 @@ fn compose(message: Option, subject: Option) -> Result<(String, subject.unwrap_or_default() ); let edited = util::open_editor(&template)?; - parse_composed(&edited).ok_or_else(|| ABORTED.to_string()) + let (subject, body) = parse_composed(&edited).ok_or_else(|| ABORTED.to_string())?; + Ok((subject, body, true)) } /// Pure parse of an edited compose file: strip `#`-comment lines, take the @@ -621,10 +701,11 @@ Second paragraph. #[test] fn compose_with_message_and_subject_succeeds_without_editor() { - let (subject, body) = + let (subject, body, from_editor) = compose(Some("body text".to_string()), Some("Subj".to_string())).unwrap(); assert_eq!(subject, "Subj"); assert_eq!(body, "body text"); + assert!(!from_editor); } #[test] @@ -827,7 +908,7 @@ Second paragraph. "test setup: no saved default" ); - let (_req, id) = build_request( + let (_req, id, _from_editor) = build_request( &profile, Some("body".to_string()), Some("Subj".to_string()), @@ -967,7 +1048,7 @@ Second paragraph. // header and for a `workspace_not_found` error message. let (_tmp, _guard) = with_temp_config_dir(); let profile = mock_profile("http://127.0.0.1:1"); - let (_req, id) = build_request( + let (_req, id, _from_editor) = build_request( &profile, Some("body".to_string()), Some("Subj".to_string()), @@ -986,7 +1067,7 @@ Second paragraph. fn build_request_resolves_the_provided_workspace() { let (_tmp, _guard) = with_temp_config_dir(); let profile = mock_profile("http://127.0.0.1:1"); - let (_req, id) = build_request( + let (_req, id, _from_editor) = build_request( &profile, Some("body".to_string()), Some("Subj".to_string()), @@ -1139,6 +1220,149 @@ Second paragraph. ); } + // --- editor draft persistence ------------------------------------------------- + + #[test] + fn save_draft_writes_subject_blank_line_body_at_0600() { + use std::os::unix::fs::PermissionsExt; + let (_tmp, _guard) = with_temp_config_dir(); + + let path = save_draft("My subject", "My body\nsecond line").unwrap(); + let content = std::fs::read_to_string(&path).unwrap(); + assert_eq!(content, "My subject\n\nMy body\nsecond line\n"); + + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } + + #[test] + fn save_draft_filename_carries_a_unix_timestamp() { + let (_tmp, _guard) = with_temp_config_dir(); + let path = save_draft("s", "b").unwrap(); + let name = path.file_name().unwrap().to_str().unwrap(); + assert!(name.starts_with("support-draft-"), "got: {name}"); + assert!(name.ends_with(".md"), "got: {name}"); + } + + #[test] + fn persist_on_editor_failure_writes_a_draft_when_send_failed_and_editor_composed() { + // Drives the exact same two steps `send_and_report` performs on a + // failed send (post, then the persist decision) without going + // through the process-exiting `handle_error` — so this can run + // in-process. The mock returning 503 twice exercises the real + // retry-once-then-give-up path in `client::support`. + let (_tmp, _guard) = with_temp_config_dir(); + let mut server = mockito::Server::new(); + let m = server + .mock("POST", "/v1/support/issues") + .with_status(503) + .expect(2) + .create(); + + let profile = mock_profile(&server.url()); + let req = SupportIssueRequest { + subject: "Composed subject".to_string(), + body: "Composed body\nsecond line".to_string(), + kind: "bug".to_string(), + severity: "high".to_string(), + context: BTreeMap::new(), + logs: None, + idempotency_key: generate_idempotency_key(), + }; + + let result = client::support::post_support_issue(&profile, None, &req); + assert!(result.is_err(), "test setup: the mock must fail the send"); + m.assert(); + + persist_on_editor_failure(&result, &req, true); + + let dir = config::config_dir().unwrap(); + let drafts: Vec<_> = std::fs::read_dir(&dir) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| { + e.file_name() + .to_string_lossy() + .starts_with("support-draft-") + }) + .collect(); + assert_eq!(drafts.len(), 1, "expected exactly one draft file"); + let content = std::fs::read_to_string(drafts[0].path()).unwrap(); + assert_eq!(content, "Composed subject\n\nComposed body\nsecond line\n"); + } + + #[test] + fn persist_on_editor_failure_is_a_noop_when_not_editor_composed() { + let (_tmp, _guard) = with_temp_config_dir(); + let req = SupportIssueRequest { + subject: "s".to_string(), + body: "b".to_string(), + kind: "bug".to_string(), + severity: "high".to_string(), + context: BTreeMap::new(), + logs: None, + idempotency_key: "k".to_string(), + }; + let result: Result<(SupportIssue, bool), SupportError> = + Err(SupportError::Connection("boom".to_string())); + + persist_on_editor_failure(&result, &req, false); + + let dir = config::config_dir().unwrap(); + let has_draft = std::fs::read_dir(&dir) + .map(|mut entries| { + entries.any(|e| { + e.ok().is_some_and(|e| { + e.file_name() + .to_string_lossy() + .starts_with("support-draft-") + }) + }) + }) + .unwrap_or(false); + assert!(!has_draft, "-m path must never write a draft"); + } + + #[test] + fn persist_on_editor_failure_is_a_noop_when_the_send_succeeded() { + let (_tmp, _guard) = with_temp_config_dir(); + let req = SupportIssueRequest { + subject: "s".to_string(), + body: "b".to_string(), + kind: "bug".to_string(), + severity: "high".to_string(), + context: BTreeMap::new(), + logs: None, + idempotency_key: "k".to_string(), + }; + let issue = SupportIssue { + public_id: "supp_1".to_string(), + status: "queued".to_string(), + subject: "s".to_string(), + kind: "bug".to_string(), + severity: "high".to_string(), + workspace_public_id: None, + created_at: "2026-09-05T00:00:00Z".to_string(), + }; + let result = Ok((issue, false)); + + persist_on_editor_failure(&result, &req, true); + + let dir = config::config_dir().unwrap(); + let has_draft = std::fs::read_dir(&dir) + .map(|mut entries| { + entries.any(|e| { + e.ok().is_some_and(|e| { + e.file_name() + .to_string_lossy() + .starts_with("support-draft-") + }) + }) + }) + .unwrap_or(false); + assert!(!has_draft, "a successful send must never write a draft"); + } + // --- report_with_profile: end-to-end against a mock server -------------------- #[test] From 1b512140ad83a61907927e03e40f5a4ce2807a01 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 11:04:14 -0700 Subject: [PATCH 11/19] test(support): drive the retry test with a zero-delay seam post_support_issue_with_delay is now pub(crate) so persist_on_editor_failure_writes_a_draft_when_send_failed_and_editor_composed can exercise the real retry-once-on-5xx path with Duration::ZERO instead of eating the real 2s RETRY_DELAY on every test run. --- src/client/support.rs | 5 ++++- src/commands/support.rs | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/client/support.rs b/src/client/support.rs index e62807f8..cd7a379f 100644 --- a/src/client/support.rs +++ b/src/client/support.rs @@ -128,7 +128,10 @@ pub fn post_support_issue( post_support_issue_with_delay(profile, workspace_id, req, RETRY_DELAY) } -fn post_support_issue_with_delay( +/// `pub(crate)` so a cross-module test (`commands::support`) can drive the +/// real retry-once path with `Duration::ZERO` instead of eating the full +/// [`RETRY_DELAY`] every run. +pub(crate) fn post_support_issue_with_delay( profile: &config::ProfileConfig, workspace_id: Option<&str>, req: &SupportIssueRequest, diff --git a/src/commands/support.rs b/src/commands/support.rs index 053f6025..1866c460 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -1250,7 +1250,9 @@ Second paragraph. // failed send (post, then the persist decision) without going // through the process-exiting `handle_error` — so this can run // in-process. The mock returning 503 twice exercises the real - // retry-once-then-give-up path in `client::support`. + // retry-once-then-give-up path in `client::support`, via the + // `pub(crate)` delay seam with `Duration::ZERO` so this doesn't eat + // the real 2s `RETRY_DELAY` on every test run. let (_tmp, _guard) = with_temp_config_dir(); let mut server = mockito::Server::new(); let m = server @@ -1270,7 +1272,12 @@ Second paragraph. idempotency_key: generate_idempotency_key(), }; - let result = client::support::post_support_issue(&profile, None, &req); + let result = client::support::post_support_issue_with_delay( + &profile, + None, + &req, + std::time::Duration::ZERO, + ); assert!(result.is_err(), "test setup: the mock must fail the send"); m.assert(); From 0a2bc9a40a9e617c451fb71b1963703797aaee52 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 11:27:21 -0700 Subject: [PATCH 12/19] fix(support): bound non-Bearer header redaction to the token only mask_authorization_header_value masked everything from the value to the end of the line, so a log line like "... missing authorization: token expired for user 42" lost its trailing message along with the credential. Bound the masked run to the same rule mask_bearer_tokens uses (up to whitespace, a quote, or end of line) and leave the rest of the line untouched. --- src/commands/support.rs | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 1866c460..2088e7e1 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -504,17 +504,27 @@ fn mask_bearer_tokens(line: &str) -> Option { } /// A bare `Authorization: ` with no `Bearer` scheme (e.g. a raw -/// `hd_...` token) — mask the whole value, keeping the header name -/// canonically capitalized (matching prior behavior) and everything before -/// it on the line untouched. `authorization:` is located anywhere in the -/// line, not just at its start. +/// `hd_...` token) — mask only the token itself, using the same boundary +/// rule as [`mask_bearer_tokens`] (the run up to whitespace, a quote, or end +/// of line), keeping the header name canonically capitalized (matching +/// prior behavior) and everything else on the line — including whatever +/// follows the token — untouched. A line like `... missing authorization: +/// token expired for user 42` must keep its message, not lose everything +/// after the value. `authorization:` is located anywhere in the line, not +/// just at its start. fn mask_authorization_header_value(line: &str) -> Option { let lower = line.to_ascii_lowercase(); let idx = lower.find("authorization:")?; let indent = &line[..idx]; let value_start = idx + "authorization:".len(); - let masked = util::mask_credential(line[value_start..].trim()); - Some(format!("{indent}Authorization: {masked}")) + let rest = &line[value_start..]; + let token_start = value_start + (rest.len() - rest.trim_start().len()); + let token_len = line[token_start..] + .find(|c: char| c.is_whitespace() || c == '"' || c == '\'') + .unwrap_or(line.len() - token_start); + let masked = util::mask_credential(&line[token_start..token_start + token_len]); + let tail = &line[token_start + token_len..]; + Some(format!("{indent}Authorization: {masked}{tail}")) } fn generate_idempotency_key() -> String { @@ -838,6 +848,17 @@ Second paragraph. assert_eq!(redact_logs(input), input); } + #[test] + fn redact_logs_non_bearer_header_masks_only_the_token_not_the_rest_of_the_line() { + let input = "2026-09-05 WARN missing authorization: token expired for user 42"; + let out = redact_logs(input); + assert!(out.contains("expired for user 42"), "got: {out}"); + assert!( + out.starts_with("2026-09-05 WARN missing Authorization: "), + "got: {out}" + ); + } + // --- idempotency key -------------------------------------------------------- #[test] From 684f7a89a2a97a28050e7f5c68056e93c1085399 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 11:27:42 -0700 Subject: [PATCH 13/19] fix(support): don't interpolate user text into the re-file hint persist_composed_report wrapped the raw subject in single quotes; an apostrophe in it would leave the shell sitting at a continuation prompt. The hint now reads both --subject and -m back from the saved draft file at re-file time (head -n 1 / tail -n +3, both paths double-quoted) instead of interpolating user-controlled text into the printed command line. --- src/commands/support.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 2088e7e1..f8d536c1 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -178,12 +178,7 @@ fn persist_on_editor_failure( /// if even that fails — print the whole thing to stderr so nothing is lost. fn persist_composed_report(subject: &str, body: &str) { match save_draft(subject, body) { - Ok(path) => { - let path = path.display(); - eprintln!( - "Your report was saved to {path}. Re-file it with: hotdata support report --subject '{subject}' -m \"$(tail -n +3 {path})\"" - ); - } + Ok(path) => eprintln!("{}", refile_hint(&path)), Err(e) => { eprintln!("warning: could not save your report to disk: {e}"); eprintln!("--- your report, so nothing is lost ---"); @@ -194,12 +189,24 @@ fn persist_composed_report(subject: &str, body: &str) { } } +/// The re-file hint printed after a draft is saved. Both `--subject` and `-m` +/// are filled by reading the draft file back at re-file time (`head`/`tail`) +/// rather than interpolating the user's own subject/body text into the +/// command line — a quote or apostrophe in either would otherwise leave the +/// shell sitting at a continuation prompt. +fn refile_hint(path: &std::path::Path) -> String { + let path = path.display(); + format!( + "Your report was saved to {path}. Re-file it with: hotdata support report --subject \"$(head -n 1 \"{path}\")\" -m \"$(tail -n +3 \"{path}\")\"" + ) +} + /// Persist a composed report to disk as `support-draft-.md` /// under the CLI config dir (mode 0600, same as the session file — the /// content is the user's own report, not a credential, but there is no /// reason to make it more visible than that). Format is `"\n\n -/// \n"`, so `tail -n +3 ` recovers the body alone (matching the -/// re-file hint in [`persist_composed_report`]). +/// \n"`, so `head -n 1 ` recovers the subject and +/// `tail -n +3 ` the body (matching [`refile_hint`]). fn save_draft(subject: &str, body: &str) -> Result { let dir = config::config_dir()?; let now = SystemTime::now() @@ -1265,6 +1272,18 @@ Second paragraph. assert!(name.ends_with(".md"), "got: {name}"); } + #[test] + fn refile_hint_reads_subject_and_body_from_the_file_rather_than_interpolating_them() { + // An apostrophe or quote in the user's own subject/body must never + // land in the printed command line — only the (trusted) path does. + let path = std::path::Path::new("/tmp/support-draft-1234567890.md"); + let hint = refile_hint(path); + assert_eq!( + hint, + "Your report was saved to /tmp/support-draft-1234567890.md. Re-file it with: hotdata support report --subject \"$(head -n 1 \"/tmp/support-draft-1234567890.md\")\" -m \"$(tail -n +3 \"/tmp/support-draft-1234567890.md\")\"" + ); + } + #[test] fn persist_on_editor_failure_writes_a_draft_when_send_failed_and_editor_composed() { // Drives the exact same two steps `send_and_report` performs on a From 5506b938123737e09eef62870fb625ee8d87a471 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sat, 5 Sep 2026 11:28:04 -0700 Subject: [PATCH 14/19] test(support): rename a test fixture token flagged by aikido hd_live_x123456789 reads like a plausible live-credential shape; swap it for an obviously-fake fixture value. --- src/commands/support.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index f8d536c1..7bd18c61 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -831,14 +831,16 @@ Second paragraph. #[test] fn redact_logs_masks_a_curl_dash_h_bearer_token_mid_line() { - let input = r#"curl -H "Authorization: Bearer hd_live_x123456789" https://api.hotdata.dev/v1/query"#; + // Obviously-fake fixture token (not a plausible live credential + // shape) so secret scanners don't flag this test fixture. + let input = r#"curl -H "Authorization: Bearer hd_notarealtoken_0001" https://api.hotdata.dev/v1/query"#; let out = redact_logs(input); assert!( - out.contains(r#"Authorization: Bearer hd_l...6789""#), + out.contains(r#"Authorization: Bearer hd_n...0001""#), "got: {out}" ); assert!(out.contains("https://api.hotdata.dev/v1/query")); - assert!(!out.contains("hd_live_x123456789")); + assert!(!out.contains("hd_notarealtoken_0001")); } #[test] From ba5a601b4cda37873cadcde3bcb58fb50043895f Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sun, 6 Sep 2026 08:11:17 -0700 Subject: [PATCH 15/19] fix(support): never lose an editor-composed report Validate --logs and --context before opening $EDITOR, and save a draft when the composed subject is the thing that's too long. Both paths previously discarded text the user had just written, since compose()'s temp file is deleted the moment it returns. --- src/commands/support.rs | 178 +++++++++++++++++++++++++++++++--------- 1 file changed, 138 insertions(+), 40 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 7bd18c61..e1dec148 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -239,17 +239,22 @@ fn build_request( ) -> Result<(SupportIssueRequest, Option, bool), String> { let (workspace_id, workspace_locked) = resolve_optional_workspace(profile, workspace_id, no_workspace)?; - let (subject, body, from_editor) = compose(message, subject)?; - validate_subject(&subject)?; + // Everything that does not depend on the composed text is validated + // BEFORE $EDITOR opens. `compose`'s temp file is deleted the moment it + // returns, so a rejection after that point throws away text the user + // just spent an editor session writing -- a mistyped --logs path or a + // --context pair missing its '=' must not cost them the report. let mut context = default_context(profile, workspace_locked); merge_user_context(&mut context, &context_pairs)?; - let logs = match logs_path { Some(path) => Some(load_logs(&path)?), None => None, }; + let (subject, body, from_editor) = compose(message, subject)?; + validate_composed_subject(&subject, &body, from_editor)?; + let req = SupportIssueRequest { subject, body, @@ -379,6 +384,21 @@ fn validate_subject(subject: &str) -> Result<(), String> { Ok(()) } +/// `validate_subject`, plus the draft rescue for the one rejection that can +/// only land after `$EDITOR` has already closed: an over-long first line. +/// Everything else `build_request` rejects is checked before composing, but +/// this check needs the composed text itself, so save it before erroring -- +/// the editor's own copy is gone by now. +fn validate_composed_subject(subject: &str, body: &str, from_editor: bool) -> Result<(), String> { + let Err(e) = validate_subject(subject) else { + return Ok(()); + }; + if from_editor { + persist_composed_report(subject, body); + } + Err(e) +} + fn default_context( profile: &config::ProfileConfig, workspace_locked: bool, @@ -674,6 +694,27 @@ mod tests { // --- parse_composed (editor compose, pure) ----------------------------- + /// Every `support-draft-*` file currently in the (temp) config dir. + fn draft_paths() -> Vec { + let dir = config::config_dir().unwrap(); + let mut paths: Vec<_> = std::fs::read_dir(&dir) + .map(|entries| { + entries + .filter_map(|e| e.ok()) + .map(|e| e.path()) + .filter(|p| { + p.file_name() + .unwrap_or_default() + .to_string_lossy() + .starts_with("support-draft-") + }) + .collect() + }) + .unwrap_or_default(); + paths.sort(); + paths + } + #[test] fn parse_composed_strips_comments_and_splits_subject_body() { let text = "\ @@ -1046,6 +1087,54 @@ Second paragraph. m.assert(); } + #[test] + fn build_request_rejects_a_bad_logs_path_before_composing() { + // Ordering guard: --logs is read before compose() runs, so an + // editor session is never spent on a report a bad path would then + // throw away. no_input keeps compose from spawning a real editor, + // and makes its failure the distinguishable one -- if compose still + // ran first this would be the TTY error instead. + let (_tmp, _guard) = with_temp_config_dir(); + util::set_no_input(true); + let err = build_request( + &mock_profile("http://127.0.0.1:1"), + None, + None, + "bug".to_string(), + "high".to_string(), + None, + true, + Some("/nonexistent/nope.log".to_string()), + vec![], + ) + .unwrap_err(); + util::set_no_input(false); + assert!(err.contains("nope.log"), "got: {err}"); + assert!(!err.contains("TTY"), "got: {err}"); + } + + #[test] + fn build_request_rejects_a_malformed_context_pair_before_composing() { + // Same ordering guard as above, for --context. + let (_tmp, _guard) = with_temp_config_dir(); + util::set_no_input(true); + let err = build_request( + &mock_profile("http://127.0.0.1:1"), + None, + None, + "bug".to_string(), + "high".to_string(), + None, + true, + None, + vec!["broken".to_string()], + ) + .unwrap_err(); + util::set_no_input(false); + assert!(err.contains("KEY=VALUE"), "got: {err}"); + assert!(!err.contains("TTY"), "got: {err}"); + } + #[test] fn build_request_non_tty_without_message_errors_before_any_http_call() { let (_tmp, _guard) = with_temp_config_dir(); @@ -1286,6 +1375,45 @@ Second paragraph. ); } + #[test] + fn over_long_editor_subject_is_saved_as_a_draft_before_erroring() { + // The editor path can't be driven end-to-end here (compose() needs a + // TTY, and spawning a real editor has no place in a test), so this + // covers the rescue at the seam build_request calls. + let (_tmp, _guard) = with_temp_config_dir(); + let subject = "x".repeat(MAX_SUBJECT_CHARS + 1); + + let err = validate_composed_subject(&subject, "Composed body", true).unwrap_err(); + + assert!(err.contains("limit 200"), "got: {err}"); + let drafts = draft_paths(); + assert_eq!(drafts.len(), 1, "expected exactly one draft file"); + assert_eq!( + std::fs::read_to_string(&drafts[0]).unwrap(), + format!("{subject}\n\nComposed body\n") + ); + } + + #[test] + fn over_long_typed_subject_errors_without_saving_a_draft() { + // --subject text is still in the caller's shell history; nothing to + // rescue, and a draft file would just be litter. + let (_tmp, _guard) = with_temp_config_dir(); + let subject = "x".repeat(MAX_SUBJECT_CHARS + 1); + + let err = validate_composed_subject(&subject, "body", false).unwrap_err(); + + assert!(err.contains("limit 200"), "got: {err}"); + assert!(draft_paths().is_empty(), "--subject path must not save one"); + } + + #[test] + fn subject_within_the_limit_saves_nothing() { + let (_tmp, _guard) = with_temp_config_dir(); + validate_composed_subject("Short enough", "body", true).unwrap(); + assert!(draft_paths().is_empty()); + } + #[test] fn persist_on_editor_failure_writes_a_draft_when_send_failed_and_editor_composed() { // Drives the exact same two steps `send_and_report` performs on a @@ -1325,18 +1453,9 @@ Second paragraph. persist_on_editor_failure(&result, &req, true); - let dir = config::config_dir().unwrap(); - let drafts: Vec<_> = std::fs::read_dir(&dir) - .unwrap() - .filter_map(|e| e.ok()) - .filter(|e| { - e.file_name() - .to_string_lossy() - .starts_with("support-draft-") - }) - .collect(); + let drafts = draft_paths(); assert_eq!(drafts.len(), 1, "expected exactly one draft file"); - let content = std::fs::read_to_string(drafts[0].path()).unwrap(); + let content = std::fs::read_to_string(&drafts[0]).unwrap(); assert_eq!(content, "Composed subject\n\nComposed body\nsecond line\n"); } @@ -1357,19 +1476,7 @@ Second paragraph. persist_on_editor_failure(&result, &req, false); - let dir = config::config_dir().unwrap(); - let has_draft = std::fs::read_dir(&dir) - .map(|mut entries| { - entries.any(|e| { - e.ok().is_some_and(|e| { - e.file_name() - .to_string_lossy() - .starts_with("support-draft-") - }) - }) - }) - .unwrap_or(false); - assert!(!has_draft, "-m path must never write a draft"); + assert!(draft_paths().is_empty(), "-m path must never write a draft"); } #[test] @@ -1397,19 +1504,10 @@ Second paragraph. persist_on_editor_failure(&result, &req, true); - let dir = config::config_dir().unwrap(); - let has_draft = std::fs::read_dir(&dir) - .map(|mut entries| { - entries.any(|e| { - e.ok().is_some_and(|e| { - e.file_name() - .to_string_lossy() - .starts_with("support-draft-") - }) - }) - }) - .unwrap_or(false); - assert!(!has_draft, "a successful send must never write a draft"); + assert!( + draft_paths().is_empty(), + "a successful send must never write a draft" + ); } // --- report_with_profile: end-to-end against a mock server -------------------- From e5aedc4469b94acba3ea31f8823efcfce5ee8a5a Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sun, 6 Sep 2026 08:12:23 -0700 Subject: [PATCH 16/19] fix(support): confirm attached logs in the output The table confirmation never mentioned --logs, so a dropped or mis-read log file was invisible until a support reply asked for it. --- src/commands/support.rs | 114 +++++++++++++++++++++++++++++++++------- 1 file changed, 95 insertions(+), 19 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index e1dec148..66cde6c0 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -150,10 +150,11 @@ fn send_and_report( from_editor: bool, output: &str, ) { + let logs_attached = req.logs.is_some(); let result = client::support::post_support_issue(profile, workspace_id.as_deref(), &req); persist_on_editor_failure(&result, &req, from_editor); match result { - Ok((issue, replay)) => print_success(&issue, replay, output), + Ok((issue, replay)) => print_success(&issue, replay, logs_attached, output), Err(e) => handle_error(&e, workspace_id.as_deref()), } } @@ -568,8 +569,10 @@ struct ReportOutput<'a> { replay: bool, } -fn print_success(issue: &SupportIssue, replay: bool, output: &str) { +fn print_success(issue: &SupportIssue, replay: bool, logs_attached: bool, output: &str) { match output { + // json/yaml stay the issue object plus `replay`: a caller parsing + // these passed --logs itself and already knows. "json" => println!( "{}", serde_json::to_string_pretty(&ReportOutput { issue, replay }).unwrap() @@ -578,27 +581,42 @@ fn print_success(issue: &SupportIssue, replay: bool, output: &str) { "{}", serde_yaml::to_string(&ReportOutput { issue, replay }).unwrap() ), - "table" => { - use crossterm::style::Stylize; - println!( - "Support request filed: {}", - issue.public_id.as_str().green() - ); - println!("Subject: {}", issue.subject); - let workspace = issue.workspace_public_id.as_deref().unwrap_or("none"); - println!( - "Severity: {} Kind: {} Workspace: {}", - issue.severity, issue.kind, workspace - ); - println!("Replies go to the email on your HotData account."); - if replay { - println!("(already filed; nothing new was sent)"); - } - } + "table" => println!("{}", success_table(issue, replay, logs_attached)), _ => unreachable!(), } } +/// The table-mode confirmation as text — pure, so what it renders is tested +/// directly rather than by capturing the process's stdout. +fn success_table(issue: &SupportIssue, replay: bool, logs_attached: bool) -> String { + use crossterm::style::Stylize; + + let mut lines = vec![ + format!( + "Support request filed: {}", + issue.public_id.as_str().green() + ), + format!("Subject: {}", issue.subject), + format!( + "Severity: {} Kind: {} Workspace: {}", + issue.severity, + issue.kind, + issue.workspace_public_id.as_deref().unwrap_or("none") + ), + ]; + // Confirm the attachment made it: --logs is the one input the user can't + // see in the response, and a silently dropped log file is the kind of + // thing they'd only discover from a support reply asking for it. + if logs_attached { + lines.push("Logs: attached".to_string()); + } + lines.push("Replies go to the email on your HotData account.".to_string()); + if replay { + lines.push("(already filed; nothing new was sent)".to_string()); + } + lines.join("\n") +} + /// The support endpoint's error envelope is Django-flat (`{"error": /// "workspace_not_found"}`) — the code IS the string, unlike the nested /// `{"error": {"code", "message"}}` shape `util::error_code` expects @@ -1510,6 +1528,64 @@ Second paragraph. ); } + // --- table rendering (pure) --------------------------------------------------- + + fn filed_issue() -> SupportIssue { + SupportIssue { + public_id: "supp_1".to_string(), + status: "queued".to_string(), + subject: "Query timing out".to_string(), + kind: "bug".to_string(), + severity: "high".to_string(), + workspace_public_id: Some("work_abc".to_string()), + created_at: "2026-09-05T00:00:00Z".to_string(), + } + } + + #[test] + fn success_table_reports_attached_logs() { + let table = success_table(&filed_issue(), false, true); + assert!(table.contains("Logs: attached"), "got: {table}"); + } + + #[test] + fn success_table_omits_the_logs_line_when_none_were_sent() { + let table = success_table(&filed_issue(), false, false); + assert!(!table.contains("Logs:"), "got: {table}"); + } + + #[test] + fn success_table_carries_the_issue_fields_and_the_replay_note() { + let plain = success_table(&filed_issue(), true, false); + assert!(plain.contains("supp_1"), "got: {plain}"); + assert!( + plain.contains("Subject: Query timing out"), + "got: {plain}" + ); + assert!( + plain.contains("Severity: high Kind: bug Workspace: work_abc"), + "got: {plain}" + ); + assert!( + plain.contains("Replies go to the email on your HotData account."), + "got: {plain}" + ); + assert!( + plain.contains("(already filed; nothing new was sent)"), + "got: {plain}" + ); + } + + #[test] + fn success_table_renders_a_missing_workspace_as_none() { + let issue = SupportIssue { + workspace_public_id: None, + ..filed_issue() + }; + let table = success_table(&issue, false, false); + assert!(table.contains("Workspace: none"), "got: {table}"); + } + // --- report_with_profile: end-to-end against a mock server -------------------- #[test] From 9ebb139898f98eb5ab16ef875e8003ff8fb7d873 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sun, 6 Sep 2026 08:13:17 -0700 Subject: [PATCH 17/19] fix(support): give each saved draft a unique name Two runs failing inside the same second wrote the same support-draft-.md, so the second silently replaced the first. --- src/commands/support.rs | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 66cde6c0..317865ca 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -202,19 +202,22 @@ fn refile_hint(path: &std::path::Path) -> String { ) } -/// Persist a composed report to disk as `support-draft-.md` +/// Persist a composed report to disk as `support-draft--.md` /// under the CLI config dir (mode 0600, same as the session file — the /// content is the user's own report, not a credential, but there is no /// reason to make it more visible than that). Format is `"\n\n /// \n"`, so `head -n 1 ` recovers the subject and -/// `tail -n +3 ` the body (matching [`refile_hint`]). +/// `tail -n +3 ` the body (matching [`refile_hint`]). The hex suffix +/// keeps two runs that fail inside the same second from overwriting each +/// other's draft; the seconds keep the names sortable by age. fn save_draft(subject: &str, body: &str) -> Result { let dir = config::config_dir()?; let now = SystemTime::now() .duration_since(UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0); - let path = dir.join(format!("support-draft-{now}.md")); + let suffix = random_hex(2); + let path = dir.join(format!("support-draft-{now}-{suffix}.md")); let content = format!("{subject}\n\n{body}\n"); util::atomic_write(&path, content.as_bytes(), 0o600)?; Ok(path) @@ -555,13 +558,18 @@ fn mask_authorization_header_value(line: &str) -> Option { Some(format!("{indent}Authorization: {masked}{tail}")) } -fn generate_idempotency_key() -> String { +/// `len` random bytes as lowercase hex (so `2 * len` characters). +fn random_hex(len: usize) -> String { use rand::RngCore; - let mut bytes = [0u8; 16]; + let mut bytes = vec![0u8; len]; rand::thread_rng().fill_bytes(&mut bytes); bytes.iter().map(|b| format!("{b:02x}")).collect() } +fn generate_idempotency_key() -> String { + random_hex(16) +} + #[derive(Serialize)] struct ReportOutput<'a> { #[serde(flatten)] @@ -1381,6 +1389,27 @@ Second paragraph. assert!(name.ends_with(".md"), "got: {name}"); } + #[test] + fn save_draft_does_not_overwrite_an_earlier_draft_from_the_same_second() { + // One process only ever writes one draft (it exits right after), so + // the collision is between two CLI runs failing in the same second. + let (_tmp, _guard) = with_temp_config_dir(); + + let first = save_draft("First", "first body").unwrap(); + let second = save_draft("Second", "second body").unwrap(); + + assert_ne!(first, second); + assert_eq!(draft_paths().len(), 2); + assert_eq!( + std::fs::read_to_string(&first).unwrap(), + "First\n\nfirst body\n" + ); + assert_eq!( + std::fs::read_to_string(&second).unwrap(), + "Second\n\nsecond body\n" + ); + } + #[test] fn refile_hint_reads_subject_and_body_from_the_file_rather_than_interpolating_them() { // An apostrophe or quote in the user's own subject/body must never From 2c336d7920cd801d6c1f8f782070b475e50a8be2 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sun, 6 Sep 2026 08:13:51 -0700 Subject: [PATCH 18/19] fix(support): show progress while filing a report The send can sit for two client timeouts plus the 2s retry backoff with nothing on screen, on exactly the failing-API case this command is for. --- src/commands/support.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/commands/support.rs b/src/commands/support.rs index 317865ca..0ff66f8b 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -151,7 +151,12 @@ fn send_and_report( output: &str, ) { let logs_attached = req.logs.is_some(); + // A failing API is the situation this command exists for, and the + // retry-once path can sit for two client timeouts plus the 2s backoff. + // Without this the terminal looks hung for the whole stretch. + let spinner = util::spinner("Filing support request..."); let result = client::support::post_support_issue(profile, workspace_id.as_deref(), &req); + spinner.finish_and_clear(); persist_on_editor_failure(&result, &req, from_editor); match result { Ok((issue, replay)) => print_success(&issue, replay, logs_attached, output), From f5f19039f7a1c9cbf5ef896b45ebe1ad3365b9e5 Mon Sep 17 00:00:00 2001 From: Zac Farrell Date: Sun, 6 Sep 2026 08:24:08 -0700 Subject: [PATCH 19/19] fix(support): rescue a composed report with no body A report typed on one line was rejected for the missing body and discarded with it, since the editor's temp file is already gone. An untouched template still aborts silently. --- src/commands/support.rs | 83 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 6 deletions(-) diff --git a/src/commands/support.rs b/src/commands/support.rs index 0ff66f8b..e83b1bde 100644 --- a/src/commands/support.rs +++ b/src/commands/support.rs @@ -352,29 +352,55 @@ fn compose( subject.unwrap_or_default() ); let edited = util::open_editor(&template)?; - let (subject, body) = parse_composed(&edited).ok_or_else(|| ABORTED.to_string())?; + let Some((subject, body)) = parse_composed(&edited) else { + return Err(abort_or_rescue(&edited)); + }; Ok((subject, body, true)) } -/// Pure parse of an edited compose file: strip `#`-comment lines, take the -/// first non-blank remaining line as the subject and everything after as the -/// body. `None` when either comes up empty — the abort case. -fn parse_composed(text: &str) -> Option<(String, String)> { +/// Split an edited compose file: strip `#`-comment lines, take the first +/// non-blank remaining line as the subject and everything after as the body. +/// Either half can come back empty; [`parse_composed`] is the caller that +/// decides an empty half is a rejection. +fn split_composed(text: &str) -> (String, String) { let mut lines = text.lines().filter(|l| !l.trim_start().starts_with('#')); let subject = loop { match lines.next() { Some(l) if l.trim().is_empty() => continue, Some(l) => break l.trim().to_string(), - None => return None, + None => break String::new(), } }; let body: String = lines.collect::>().join("\n").trim().to_string(); + (subject, body) +} + +/// Pure parse of an edited compose file. `None` when either half comes up +/// empty — the abort case. +fn parse_composed(text: &str) -> Option<(String, String)> { + let (subject, body) = split_composed(text); if subject.is_empty() || body.is_empty() { return None; } Some((subject, body)) } +/// What `compose` returns when `parse_composed` rejects the edited file. +/// An untouched or emptied template is the documented cancel — stay silent +/// and leave no draft behind. Anything the user actually typed was rejected +/// only for the missing half, and `open_editor`'s temp file is already +/// gone, so rescue that text the same way a failed send does. +fn abort_or_rescue(edited: &str) -> String { + let (subject, body) = split_composed(edited); + if subject.is_empty() { + // No non-comment content at all, so nothing was typed: `split_composed` + // only leaves the subject empty when it found no content line. + return ABORTED.to_string(); + } + persist_composed_report(&subject, &body); + "a report needs a subject line and a body; nothing was sent".to_string() +} + /// Truncate to `max` chars (not bytes), respecting UTF-8 boundaries. fn truncate_chars(s: &str, max: usize) -> String { s.chars().take(max).collect() @@ -782,6 +808,51 @@ Second paragraph. // --- compose (message/subject validation) ------------------------------- + #[test] + fn a_typed_report_with_no_body_line_is_rescued_to_a_draft() { + // parse_composed rejects subject-only text, and open_editor's temp + // file is gone by then -- so this is the same class of loss as a + // failed send, and needs the same rescue. + let (_tmp, _guard) = with_temp_config_dir(); + let edited = "Everything is broken please help\n\n\ + # Lines starting with '#' are ignored.\n"; + + let err = abort_or_rescue(edited); + + assert_ne!(err, ABORTED, "typed text must not abort silently"); + let drafts = draft_paths(); + assert_eq!(drafts.len(), 1, "expected exactly one draft file"); + assert_eq!( + std::fs::read_to_string(&drafts[0]).unwrap(), + "Everything is broken please help\n\n\n" + ); + } + + #[test] + fn an_untouched_template_aborts_silently_without_a_draft() { + // "empty to abort" is the documented cancel; it must not litter the + // config dir with a draft of nothing. + let (_tmp, _guard) = with_temp_config_dir(); + let edited = "\n\n\ + # Lines starting with '#' are ignored. First non-comment line is the subject,\n\ + # the rest is the report body. Save and quit to send; empty to abort.\n"; + + assert_eq!(abort_or_rescue(edited), ABORTED); + assert!(draft_paths().is_empty()); + } + + #[test] + fn split_composed_keeps_an_empty_half_that_parse_composed_would_reject() { + assert_eq!( + split_composed("Just a subject\n# a comment\n"), + ("Just a subject".to_string(), String::new()) + ); + assert_eq!( + split_composed("# only comments\n"), + (String::new(), String::new()) + ); + } + #[test] fn compose_with_message_but_no_subject_errors() { let err = compose(Some("body text".to_string()), None).unwrap_err();