From cf646d7e4080bf6d0f9f23d96fc3fd10f763ea39 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Mon, 28 Sep 2026 14:21:17 +0300 Subject: [PATCH 1/4] Record command history and add mapbox history Each run appends one line of execution metadata to ~/.mapbox/history/.jsonl, kept 30 days: the command path from the command tree, invocation, exit code, error code, duration, request count and the last five request ids. Never an argument value, URL, error message or account: arguments carry search terms, file paths and ids. The file is written through dated_jsonl: private, one file per UTC day, pruned to the retention window. On by default. `mapbox config set history off` turns it off for good, MAPBOX_HISTORY=0 or =1 for a session over the setting. With it off, nothing is written and no directory is created. With it on, the config directory is created when missing, so history works the same for someone who only ever set MAPBOX_ACCESS_TOKEN. Not recorded: --help, --version, completion, history itself, and runs under sudo, whose root-owned files would stop the user's own runs appending (aws/aws-cli#10031). `mapbox history list` shows the most recent runs, newest first, and `mapbox history show [id]` one run, the newest by default, found by any prefix of its id. The read-only-command tests that held ~/.mapbox untouched now check both sides: with history on only `history/` may appear, with it off nothing. --- CHANGELOG.md | 9 ++ README.md | 21 ++++ docs/commands.md | 164 +++++++++++++++++++++++++-- src/account_usage.rs | 2 +- src/config.rs | 29 ++++- src/dated_jsonl.rs | 196 ++++++++++++++++++++++++++++++++ src/history.rs | 239 +++++++++++++++++++++++++++++++++++++++ src/main.rs | 13 +++ src/run_history.rs | 165 +++++++++++++++++++++++++++ src/run_record.rs | 28 ++++- src/schema.rs | 8 ++ src/telemetry.rs | 19 +++- tests/auth_profiles.rs | 62 +++++++--- tests/completion.rs | 16 ++- tests/config.rs | 14 ++- tests/history.rs | 239 +++++++++++++++++++++++++++++++++++++++ tests/non_interactive.rs | 46 ++++++-- tests/source_guards.rs | 3 + 18 files changed, 1216 insertions(+), 57 deletions(-) create mode 100644 src/dated_jsonl.rs create mode 100644 src/history.rs create mode 100644 src/run_history.rs create mode 100644 tests/history.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c236924..5448635 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,15 @@ that may never merge. They are not releases and are not listed here. ## Unreleased +- Command history, on by default: each run appends one line to + `~/.mapbox/history/.jsonl`, kept 30 days, with its command path, exit + code, error code, duration and request ids — never an argument value. It + stays on your machine. `mapbox history list` and `mapbox history show` + read it back. Turn it off with `mapbox config set history off` or + `MAPBOX_HISTORY=0`. A script sees no change on stdout, stderr or the exit + code; it does find a `~/.mapbox/history` directory it didn't before, and + `mapbox config list` now reports a second key, `history`. + - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API parameter this CLI's specs don't declare a flag for. diff --git a/README.md b/README.md index 714d97e..c5d3df4 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ time from OpenAPI specs, so they always match the specs. - [`--schema`](#--schema) - [Confirmation and `--yes`](#confirmation-and---yes) - [Update notices](#update-notices) + - [Command history](#command-history) - [Privacy](#privacy) - [Uninstall](#uninstall) - [Contributing](#contributing) @@ -438,6 +439,26 @@ between runs. A build that names no release channel never checks at all, and see [Config](docs/commands.md#config) — rather than just the session an environment variable happens to be set in. +### Command history + +Each run appends one line to `~/.mapbox/history/.jsonl` (or under +`$MAPBOX_CONFIG_DIR`), kept for 30 days: which command ran (its command path, +like `search forward`), how it ended, how long it took and the request ids +support can look up. Argument values are never recorded — not what you +searched for, not a file path, not a token. The files are readable only by +you and never leave your machine. + +```sh +mapbox history list # the most recent runs, newest first +mapbox history show # everything recorded about the newest run +mapbox history show be40d711 # or one run, by any prefix of its id +``` + +`--help`, `--version`, `completion`, `history` itself and runs under `sudo` +are not recorded. `mapbox config set history off` turns history off for +good, and `MAPBOX_HISTORY=0` for one shell; with it off, nothing is written +and no directory is created. `MAPBOX_CLI_NO_TELEMETRY` does not affect it. + ### Privacy **YOUR PRIVACY - COLLECTION OF TELEMETRY** diff --git a/docs/commands.md b/docs/commands.md index 2c2ca03..aa8f15f 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -64,6 +64,9 @@ nests, and is typed `mapbox styles draft get`. [config.set](#mapbox-config-set) · [config.list](#mapbox-config-list) · [config.unset](#mapbox-config-unset) +**[History](#history)** — [history.list](#mapbox-history-list) · +[history.show](#mapbox-history-show) + **[Doctor](#doctor)** — [doctor](#mapbox-doctor) **[Usage](#usage)** — [usage](#mapbox-usage) @@ -3194,10 +3197,14 @@ Removed /home/user/.local/bin/mapbox. ## Config Settings that persist across shells and sessions — `~/.mapbox/config.json` -(or `$MAPBOX_CONFIG_DIR`), written the same way credentials are. One setting -today, `update-check`, which mirrors `MAPBOX_NO_UPDATE_CHECK` (see [Update -notices](../README.md#update-notices)) but stays off in every future shell -rather than only the one the environment variable was set in. +(or `$MAPBOX_CONFIG_DIR`), written the same way credentials are. Each is the +persisted form of an environment variable that only lasts for the shell it +was set in, and stays in every future shell instead. + +| Key | Default | What it controls | +| --- | --- | --- | +| `update-check` | `on` | The update notice; mirrors `MAPBOX_NO_UPDATE_CHECK` (see [Update notices](../README.md#update-notices)) | +| `history` | `on` | [Command history](../README.md#command-history), read by `mapbox history`; `MAPBOX_HISTORY=0` or `=1` overrides it for a session | ### `mapbox config get` @@ -3209,7 +3216,7 @@ than failing, the same forgiving read the update-check cache itself uses. | Parameter | Effect | | --- | --- | -| `` | Which setting to read. Only `update-check` exists today. | +| `` | Which setting to read: `update-check` or `history`. | #### Examples @@ -3248,7 +3255,7 @@ without an environment variable. | Parameter | Effect | | --- | --- | -| `` | Which setting to change. Only `update-check` exists today. | +| `` | Which setting to change: `update-check` or `history`. | | `` | `on` or `off`. | #### Examples @@ -3305,6 +3312,7 @@ mapbox config list ``` update-check on +history on ``` @@ -3314,6 +3322,10 @@ update-check on { "key": "update-check", "value": true + }, + { + "key": "history", + "value": true } ] ``` @@ -3332,7 +3344,7 @@ default, a key explicitly set to the old default value does not. | Parameter | Effect | | --- | --- | -| `` | Which setting to clear. Only `update-check` exists today. | +| `` | Which setting to clear: `update-check` or `history`. | #### Examples @@ -3364,6 +3376,144 @@ update-check cleared, now on (default). --- +## History + +The runs [command history](../README.md#command-history) recorded on this +machine over the last 30 days: which command ran, how it ended, how long it +took and the request ids support can look up. Argument values are never +recorded, so a run shows as its command path — `mapbox search forward`, +not what was searched for. History is on by default; with it off +(`mapbox config set history off`), both commands find nothing and say why +on stderr. Neither makes a request or needs a token, and neither is itself +recorded — nor are `--help`, `--version`, `completion` or a run under +`sudo`. + +### `mapbox history list` + +The most recent runs, newest first: a short id, when it ran (UTC), its exit +code and its command path. `json` gives each run's full `id`, which +`history show` also accepts shortened to any prefix that names one run. + +#### Parameters + +| Parameter | Effect | +| --- | --- | +| `--limit ` | How many runs to list. Defaults to `20`; `0` lists every recorded run. | + +#### Examples + +```sh +mapbox history list + +mapbox history list --limit 0 +``` + +#### Outputs + + + + +
textjson
+ +``` +d05b3f4d 2026-09-28T11:20:03.095Z 2 mapbox styles +be40d711 2026-09-28T11:20:03.045Z 1 mapbox styles list +``` + + + +```json +[ + { + "command": [ + "styles" + ], + "durationMs": 41, + "errorCode": "usage", + "exitCode": 2, + "id": "d05b3f4d-9947-4662-a037-3d00b68d1d6e", + "time": "2026-09-28T11:20:03.095Z" + }, + { + "command": [ + "styles", + "list" + ], + "durationMs": 157, + "errorCode": "http_401", + "exitCode": 1, + "id": "be40d711-3d62-4e9b-8dde-23535020b368", + "time": "2026-09-28T11:20:03.045Z" + } +] +``` + +
+ +### `mapbox history show` + +Everything recorded about one run: its command path, how it ended, its +error code, how many requests it made and the ids of the last five. `json` +gives the record as it was written. An id that names no run fails with +`history_not_found`; a prefix shared by several fails with +`history_ambiguous_id`; with nothing recorded yet, `show` with no id fails +with `history_empty`. + +#### Parameters + +| Parameter | Effect | +| --- | --- | +| `[id]` | The run's id, or any prefix of it that names one run. The newest run when left out. | + +#### Examples + +```sh +mapbox history show + +mapbox history show be40d711 +``` + +#### Outputs + + + + +
textjson
+ +``` +Run be40d711-3d62-4e9b-8dde-23535020b368 +Time 2026-09-28T11:20:03.045Z (mapbox 0.3.0) +Command mapbox styles list +Exit 1 after 157 ms +Error http_401 +Requests 1 + request id 7ovbf8wEjg4S_uS-u8SNW0OHb64pCVgD5fThd2C2q9ZlE7bDY-s0yw== +``` + + + +```json +{ + "command": [ + "styles", + "list" + ], + "durationMs": 157, + "errorCode": "http_401", + "exitCode": 1, + "id": "be40d711-3d62-4e9b-8dde-23535020b368", + "invocation": "execute", + "requestCount": 1, + "requestIds": [ + "7ovbf8wEjg4S_uS-u8SNW0OHb64pCVgD5fThd2C2q9ZlE7bDY-s0yw==" + ], + "time": "2026-09-28T11:20:03.045Z", + "version": "0.3.0" +} +``` + +
+ ## Doctor ### `mapbox doctor` diff --git a/src/account_usage.rs b/src/account_usage.rs index ee5c106..113ed42 100644 --- a/src/account_usage.rs +++ b/src/account_usage.rs @@ -508,7 +508,7 @@ fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { } /// The inverse of [`days_from_civil`]. -fn civil_from_days(z: i64) -> (i64, u32, u32) { +pub(crate) fn civil_from_days(z: i64) -> (i64, u32, u32) { let z = z + 719468; let era = z.div_euclid(146097); let doe = z - era * 146097; // [0, 146096] diff --git a/src/config.rs b/src/config.rs index af955ea..09b9cce 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,9 +6,10 @@ //! file beside the credentials, written through the same //! [`crate::auth::write_private`] so it gets the same `0600` treatment. //! -//! One setting today — `update-check` — with room for more: `get`/`set`/ -//! `unset` take a `key`, restricted by clap to [`KEYS`], so adding a second -//! setting is a new key and a new match arm rather than a new subcommand. +//! Two settings — `update-check` and `history` (see [`crate::run_history`]). +//! `get`/`set`/`unset` take a `key`, restricted by clap to [`KEYS`], so +//! adding a setting is a new key and a new match arm rather than a new +//! subcommand. //! `list` needs no key at all: it walks [`KEYS`] and reports every setting's //! current value in one call, which `get` cannot — the whole reason it //! exists alongside `get`/`set` rather than waiting for a second setting to @@ -30,7 +31,8 @@ pub const COMMAND: &str = "config"; const CONFIG_FILE: &str = "config.json"; const UPDATE_CHECK_KEY: &str = "update-check"; -const KEYS: &[&str] = &[UPDATE_CHECK_KEY]; +const HISTORY_KEY: &str = "history"; +const KEYS: &[&str] = &[UPDATE_CHECK_KEY, HISTORY_KEY]; const ON: &str = "on"; const OFF: &str = "off"; @@ -43,6 +45,8 @@ const OFF: &str = "off"; struct Config { #[serde(default, skip_serializing_if = "Option::is_none")] update_check: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + history: Option, } fn config_path() -> Option { @@ -83,6 +87,12 @@ pub fn update_check_enabled() -> bool { update_check_setting(&read_config()) } +/// Whether [`crate::run_history`] records runs, per the persisted setting. +/// On unless turned off. +pub fn history_enabled() -> bool { + read_config().history.unwrap_or(true) +} + fn on_off(enabled: bool) -> &'static str { if enabled { ON @@ -97,6 +107,7 @@ fn on_off(enabled: bool) -> &'static str { fn resolve(config: &Config, key: &str) -> bool { match key { UPDATE_CHECK_KEY => update_check_setting(config), + HISTORY_KEY => config.history.unwrap_or(true), _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"), } } @@ -109,6 +120,7 @@ fn resolve(config: &Config, key: &str) -> bool { fn clear(config: &mut Config, key: &str) { match key { UPDATE_CHECK_KEY => config.update_check = None, + HISTORY_KEY => config.history = None, _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"), } } @@ -173,6 +185,7 @@ pub fn set(matches: &ArgMatches, mode: Mode) -> Result<()> { let mut config = read_config(); match key.as_str() { UPDATE_CHECK_KEY => config.update_check = Some(enabled), + HISTORY_KEY => config.history = Some(enabled), _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"), } write_config(&config)?; @@ -228,6 +241,7 @@ mod tests { fn the_config_round_trips_and_tolerates_an_empty_one() { let off = Config { update_check: Some(false), + ..Config::default() }; let text = serde_json::to_string(&off).expect("serialize"); assert_eq!(text, r#"{"update_check":false}"#); @@ -249,7 +263,10 @@ mod tests { #[test] fn resolve_matches_update_check_setting_at_every_state() { for update_check in [None, Some(true), Some(false)] { - let config = Config { update_check }; + let config = Config { + update_check, + ..Config::default() + }; assert_eq!( resolve(&config, UPDATE_CHECK_KEY), update_check_setting(&config) @@ -265,6 +282,7 @@ mod tests { fn clear_removes_the_key_rather_than_writing_the_default() { let mut explicit_default = Config { update_check: Some(true), + ..Config::default() }; clear(&mut explicit_default, UPDATE_CHECK_KEY); assert_eq!(explicit_default, Config::default()); @@ -272,6 +290,7 @@ mod tests { let mut explicit_off = Config { update_check: Some(false), + ..Config::default() }; clear(&mut explicit_off, UPDATE_CHECK_KEY); assert_eq!(explicit_off.update_check, None); diff --git a/src/dated_jsonl.rs b/src/dated_jsonl.rs new file mode 100644 index 0000000..31375f4 --- /dev/null +++ b/src/dated_jsonl.rs @@ -0,0 +1,196 @@ +//! Private, append-only, one-file-per-UTC-day JSONL directories under the +//! config directory, pruned to a fixed number of days, for the consumers of +//! [`crate::run_record`] that keep records on disk. The only files this +//! deletes are ones named exactly `YYYY-MM-DD.jsonl` inside the directory +//! it was handed. + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use crate::auth; + +/// `/`, created `0700`, or `None`. +/// +/// A missing config directory is created `0700`; an existing one is left +/// as it is, since a record may come from a read-only command and hardening +/// it is `auth`'s job. A config path that isn't a directory is `auth`'s to +/// report. +pub(crate) fn private_dir(name: &str) -> Option { + let config = auth::config_dir_path()?; + if config.exists() && !config.is_dir() { + return None; + } + let dir = config.join(name); + let mut builder = std::fs::DirBuilder::new(); + builder.recursive(true); + #[cfg(unix)] + { + use std::os::unix::fs::{DirBuilderExt, PermissionsExt}; + builder.mode(0o700); + builder.create(&dir).ok()?; + // `mode` is filtered by the umask and skipped for a directory that + // already existed. + let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); + } + #[cfg(not(unix))] + builder.create(&dir).ok()?; + Some(dir) +} + +/// Appends `line` to today's file in `dir`, and on the first write of a day +/// deletes files older than `keep_days` (today included). Best-effort. +pub(crate) fn append(dir: &Path, line: &str, keep_days: u64) { + let now = now_secs(); + let (today, _) = utc_date(now); + let path = dir.join(format!("{today}.jsonl")); + let is_new_day = !path.exists(); + // One `write` per line. `O_APPEND` places each one at the end, but a + // line past the platform's atomic-write size is not guaranteed to stay + // whole against a parallel run writing at the same moment. + if let Ok(mut file) = open_private(&path) { + let _ = file.write_all(format!("{line}\n").as_bytes()); + } + if is_new_day { + prune(dir, now, keep_days); + } +} + +/// Every line in `dir`'s dated files, oldest first. A missing directory is +/// no lines. +pub(crate) fn read_all(dir: &Path) -> Vec { + let Ok(entries) = std::fs::read_dir(dir) else { + return vec![]; + }; + let mut names: Vec = entries + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| dated_file(name).is_some()) + .collect(); + names.sort(); + names + .iter() + .filter_map(|name| std::fs::read_to_string(dir.join(name)).ok()) + .flat_map(|text| { + text.lines() + .filter(|line| !line.is_empty()) + .map(str::to_string) + .collect::>() + }) + .collect() +} + +/// Opens `path` for appending, created `0600` if missing. +fn open_private(path: &Path) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + options.append(true).create(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + +fn prune(dir: &Path, now: u64, keep_days: u64) { + let (oldest_kept, _) = utc_date(now.saturating_sub(keep_days.saturating_sub(1) * 86_400)); + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name().to_string_lossy().into_owned(); + if let Some(date) = dated_file(&name) { + if date < oldest_kept.as_str() { + let _ = std::fs::remove_file(dir.join(&name)); + } + } + } +} + +fn dated_file(name: &str) -> Option<&str> { + let date = name.strip_suffix(".jsonl")?; + let shape = date.len() == 10 + && date.char_indices().all(|(i, c)| match i { + 4 | 7 => c == '-', + _ => c.is_ascii_digit(), + }); + shape.then_some(date) +} + +/// `YYYY-MM-DD` and the seconds into that day, in UTC. +fn utc_date(unix_secs: u64) -> (String, u64) { + let days = (unix_secs / 86_400) as i64; + let (y, m, d) = crate::account_usage::civil_from_days(days); + (format!("{y:04}-{m:02}-{d:02}"), unix_secs % 86_400) +} + +/// RFC 3339 in UTC, to the millisecond. +pub(crate) fn timestamp(at: SystemTime) -> String { + let since = at.duration_since(UNIX_EPOCH).unwrap_or_default(); + let (date, secs) = utc_date(since.as_secs()); + format!( + "{date}T{:02}:{:02}:{:02}.{:03}Z", + secs / 3600, + secs % 3600 / 60, + secs % 60, + since.subsec_millis() + ) +} + +fn now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |d| d.as_secs()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_dated_files_are_pruned() { + assert_eq!(dated_file("2026-09-24.jsonl"), Some("2026-09-24")); + for name in [ + "user-id", + "last-version", + "2026-09-24.json", + "notes.jsonl", + "2026-9-24.jsonl", + ] { + assert_eq!(dated_file(name), None, "{name}"); + } + } + + #[test] + fn prune_keeps_seven_days_and_nothing_else_is_touched() { + let dir = + std::env::temp_dir().join(format!("mapbox-dated-prune-{}", rand::random::())); + std::fs::create_dir_all(&dir).unwrap(); + for name in [ + "2026-09-17.jsonl", + "2026-09-18.jsonl", + "2026-09-24.jsonl", + "user-id", + "2026-09-01.txt", + ] { + std::fs::write(dir.join(name), "x").unwrap(); + } + // 2026-09-24T12:00:00Z: 09-18 through 09-24 is seven days. + prune(&dir, 1_790_251_200, 7); + let mut left: Vec = std::fs::read_dir(&dir) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + left.sort(); + std::fs::remove_dir_all(&dir).unwrap(); + assert_eq!( + left, + [ + "2026-09-01.txt", + "2026-09-18.jsonl", + "2026-09-24.jsonl", + "user-id" + ] + ); + } +} diff --git a/src/history.rs b/src/history.rs new file mode 100644 index 0000000..421a00b --- /dev/null +++ b/src/history.rs @@ -0,0 +1,239 @@ +//! `mapbox history` — the runs [`crate::run_history`] recorded. +//! +//! `list` is one line per run, newest first; `show` is everything recorded +//! about one run, the newest when no id is given. An id can be shortened to +//! any prefix that names one run, the way `list` prints them. +//! +//! Reads history and nothing else: no token, no request, and nothing +//! created on disk. It is not itself recorded. + +use anyhow::Result; +use clap::{value_parser, Arg, ArgMatches, Command}; +use serde_json::Value; + +use crate::output::{self, CliError, Mode}; +use crate::remedy::Remedy; +use crate::run_history; + +pub const COMMAND: &str = "history"; + +const DEFAULT_LIMIT: usize = 20; +/// How much of an id `list` prints: enough to tell runs apart, short enough +/// to type back into `show`. +const SHORT_ID: usize = 8; + +pub fn command() -> Command { + Command::new(COMMAND) + .about("List and show recent command runs") + .long_about( + "List and show recent command runs: which command ran, how it ended and \ + how long it took, kept for 30 days on this machine. Argument values are \ + never recorded. Turn it off with `mapbox config set history off`.", + ) + .subcommand_required(true) + .subcommand( + Command::new("list") + .about("List the most recent runs, newest first") + .arg( + Arg::new("limit") + .long("limit") + .value_parser(value_parser!(usize)) + .default_value(DEFAULT_LIMIT.to_string()) + .help("How many runs to list; 0 lists every run recorded"), + ), + ) + .subcommand( + Command::new("show") + .about("Show everything recorded about one run") + .arg( + Arg::new("id") + .help("The run's id, or a prefix of it; the newest run when left out"), + ), + ) +} + +pub fn list(matches: &ArgMatches, mode: Mode) -> Result<()> { + let limit = *matches.get_one::("limit").expect("has a default"); + let mut entries = run_history::entries(); + entries.reverse(); + if limit > 0 { + entries.truncate(limit); + } + if entries.is_empty() { + hint_when_off(); + } + + let text = entries + .iter() + .map(|entry| { + format!( + "{} {} {:>4} {}", + short_id(entry), + field(entry, "time"), + exit_code(entry), + command_line(entry) + ) + }) + .collect::>() + .join("\n"); + let json = entries + .iter() + .map(|entry| { + let mut summary = serde_json::Map::new(); + for key in [ + "id", + "time", + "command", + "exitCode", + "errorCode", + "durationMs", + ] { + if let Some(value) = entry.get(key) { + summary.insert(key.to_string(), value.clone()); + } + } + Value::Object(summary) + }) + .collect(); + + output::emit(mode, &text, Value::Array(json)) +} + +pub fn show(matches: &ArgMatches, mode: Mode) -> Result<()> { + let entries = run_history::entries(); + let entry = match matches.get_one::("id") { + None => entries.last().cloned().ok_or_else(|| { + hint_when_off(); + CliError::new("history_empty", "No runs have been recorded yet.") + })?, + Some(prefix) => find(&entries, prefix)?, + }; + output::emit(mode, &detail(&entry), entry) +} + +/// The one run whose id starts with `prefix`. +fn find(entries: &[Value], prefix: &str) -> Result { + let matching: Vec<&Value> = entries + .iter() + .filter(|entry| !prefix.is_empty() && field(entry, "id").starts_with(prefix)) + .collect(); + match matching.as_slice() { + [one] => Ok((*one).clone()), + [] => Err(CliError::new( + "history_not_found", + format!("No recorded run has an id starting with `{prefix}`."), + ) + .with_remedy(Remedy::default().with_action(Some("mapbox history list".to_string()))) + .into()), + many => Err(CliError::new( + "history_ambiguous_id", + format!( + "`{prefix}` starts {} run ids; give more of the id.", + many.len() + ), + ) + .into()), + } +} + +/// A person-readable view of one run. JSON gets the line as it was recorded. +fn detail(entry: &Value) -> String { + let mut out = vec![ + format!("Run {}", field(entry, "id")), + format!( + "Time {} (mapbox {})", + field(entry, "time"), + field(entry, "version") + ), + format!("Command {}", command_line(entry)), + format!( + "Exit {} after {} ms", + exit_code(entry), + entry["durationMs"].as_u64().unwrap_or(0) + ), + ]; + if let Some(code) = entry.get("errorCode").and_then(Value::as_str) { + out.push(format!("Error {code}")); + } + if let Some(count) = entry.get("requestCount").and_then(Value::as_u64) { + out.push(format!("Requests {count}")); + } + for id in entry["requestIds"].as_array().into_iter().flatten() { + if let Some(id) = id.as_str() { + out.push(format!(" request id {id}")); + } + } + out.join("\n") +} + +/// Says why there is nothing to read, on stderr, when history is off. +fn hint_when_off() { + if !run_history::enabled() { + output::progress( + "History is off. Turn it on with `mapbox config set history on`, \ + or `MAPBOX_HISTORY=1` for this shell.", + ); + } +} + +fn field<'a>(value: &'a Value, key: &str) -> &'a str { + value.get(key).and_then(Value::as_str).unwrap_or("-") +} + +fn short_id(entry: &Value) -> String { + field(entry, "id").chars().take(SHORT_ID).collect() +} + +fn exit_code(entry: &Value) -> String { + entry + .get("exitCode") + .and_then(Value::as_u64) + .map_or_else(|| "-".to_string(), |code| code.to_string()) +} + +/// `mapbox` and the command path: what ran, never what was typed after it. +fn command_line(entry: &Value) -> String { + let path: Vec<&str> = entry["command"] + .as_array() + .map(|words| words.iter().filter_map(Value::as_str).collect()) + .unwrap_or_default(); + if path.is_empty() { + "mapbox".to_string() + } else { + format!("mapbox {}", path.join(" ")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn runs() -> Vec { + vec![ + json!({ "id": "abc12345-0000-4000-8000-000000000001" }), + json!({ "id": "abc19999-0000-4000-8000-000000000002" }), + json!({ "id": "def00000-0000-4000-8000-000000000003" }), + ] + } + + #[test] + fn a_prefix_that_names_one_run_finds_it() { + let found = find(&runs(), "abc1234").unwrap(); + assert_eq!(field(&found, "id"), "abc12345-0000-4000-8000-000000000001"); + } + + #[test] + fn a_prefix_that_names_several_or_none_is_an_error() { + let code = |prefix: &str| { + find(&runs(), prefix) + .unwrap_err() + .downcast::() + .unwrap() + .code + }; + assert_eq!(code("abc1"), "history_ambiguous_id"); + assert_eq!(code("fff"), "history_not_found"); + assert_eq!(code(""), "history_not_found"); + } +} diff --git a/src/main.rs b/src/main.rs index 41ce04b..52319c7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,14 +19,17 @@ mod auth; mod completion; mod config; mod confirm; +mod dated_jsonl; mod deprecation; mod doctor; mod executor; mod generate_skills; +mod history; mod http; mod link; mod output; mod remedy; +mod run_history; mod run_record; mod schema; mod skill_dest; @@ -609,6 +612,9 @@ fn build_app(specs: &[ServiceSpec]) -> Command { // machine, never the network. app = app.subcommand(config::command()); + // Beside `config`, which turns the history it reads on and off. + app = app.subcommand(history::command()); + // Reads what the other hand-written commands above also read — the // token store, the proxy environment, the config and telemetry // switches — so it belongs beside them rather than the API surface @@ -1108,6 +1114,13 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - Some(("unset", unset_matches)) => config::unset(unset_matches, mode)?, _ => unreachable!("`config` sets subcommand_required(true)"), }, + // Ahead of the generic service arm too: it reads local history and + // makes no request. + Some((history::COMMAND, history_matches)) => match history_matches.subcommand() { + Some(("list", list_matches)) => history::list(list_matches, mode)?, + Some(("show", show_matches)) => history::show(show_matches, mode)?, + _ => unreachable!("`history` sets subcommand_required(true)"), + }, // Also ahead of the generic service arm: read-only except for the // opt-in `--verify` request, and needs no credential load of its own // — it reports what one would resolve to, not what a fresh one diff --git a/src/run_history.rs b/src/run_history.rs new file mode 100644 index 0000000..088da10 --- /dev/null +++ b/src/run_history.rs @@ -0,0 +1,165 @@ +//! Command history: one line of execution metadata per run in +//! `~/.mapbox/history/.jsonl` (or under `$MAPBOX_CONFIG_DIR`), +//! kept for [`RETENTION_DAYS`] days and read back by `mapbox history`. +//! +//! On by default, so it keeps only what is safe to keep without anyone +//! having asked: the command path from the command tree (`search forward`, +//! never what was typed after it), how the run ended, how long it took and +//! the request ids support can look up. No argument values, URLs, error +//! messages or account — arguments carry search terms, file paths and ids. +//! +//! `mapbox config set history off` turns it off, `MAPBOX_HISTORY=0` or `=1` +//! for a session over the setting. Turned off, nothing is created on disk. +//! Turned on, the config directory is created if it is missing, so history +//! works the same for someone who only ever set `MAPBOX_ACCESS_TOKEN`. +//! +//! Not recorded: +//! - `--help` and `--version`, which answer a question rather than run a +//! command; +//! - `history` itself, which would push out what it was reading; +//! - a run under `sudo`, whose files would belong to root inside the +//! user's home and stop the user's own runs appending to them — the +//! failure AWS CLI shipped in 2.33.9 (aws/aws-cli#10031); +//! - `completion`, which [`crate::run_record`] already skips. + +use std::path::PathBuf; +use std::time::SystemTime; + +use serde::Serialize; + +use crate::run_record::{Invocation, Record}; +use crate::{auth, config, dated_jsonl, history, telemetry}; + +const DIR: &str = "history"; +const HISTORY_ENV: &str = "MAPBOX_HISTORY"; +pub(crate) const RETENTION_DAYS: u64 = 30; +/// The last few are enough to hand to support; a paginated run can make +/// hundreds of requests. +const MAX_REQUEST_IDS: usize = 5; + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct Line { + id: String, + time: String, + version: &'static str, + #[serde(skip_serializing_if = "Vec::is_empty")] + command: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + invocation: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + duration_ms: u64, + #[serde(skip_serializing_if = "is_zero")] + request_count: usize, + #[serde(skip_serializing_if = "Vec::is_empty")] + request_ids: Vec, +} + +fn is_zero(n: &usize) -> bool { + *n == 0 +} + +/// Appends the run's line, unless history is off or the run is one it +/// does not record. +pub(crate) fn write(record: &Record) { + if !enabled() || !recorded(record, std::env::var_os("SUDO_USER").is_some()) { + return; + } + let Ok(text) = serde_json::to_string(&line(record)) else { + return; + }; + if let Some(dir) = dated_jsonl::private_dir(DIR) { + dated_jsonl::append(&dir, &text, RETENTION_DAYS); + } +} + +/// Whether this run records history: `MAPBOX_HISTORY` when it is set, the +/// persisted setting otherwise. +pub(crate) fn enabled() -> bool { + telemetry::env_switch(HISTORY_ENV).unwrap_or_else(config::history_enabled) +} + +fn recorded(record: &Record, under_sudo: bool) -> bool { + !under_sudo + && !matches!( + record.invocation, + Some(Invocation::Help | Invocation::Version) + ) + && record.command.first().map(String::as_str) != Some(history::COMMAND) +} + +fn line(record: &Record) -> Line { + let ids: Vec = record + .requests + .iter() + .filter_map(|request| request.request_id.clone()) + .collect(); + Line { + id: record.id.clone(), + time: dated_jsonl::timestamp(SystemTime::now()), + version: env!("CARGO_PKG_VERSION"), + command: record.command.clone(), + invocation: record.invocation.map(Invocation::as_str), + exit_code: record.exit_code, + error_code: record.error.as_ref().map(|error| error.code.clone()), + duration_ms: record.duration.as_millis() as u64, + request_count: record.requests.len(), + request_ids: ids[ids.len().saturating_sub(MAX_REQUEST_IDS)..].to_vec(), + } +} + +/// Where history lives, without creating it. +fn dir_path() -> Option { + Some(auth::config_dir_path()?.join(DIR)) +} + +/// Every run in history, oldest first, skipping any line that doesn't parse. +pub(crate) fn entries() -> Vec { + let Some(dir) = dir_path() else { + return vec![]; + }; + dated_jsonl::read_all(&dir) + .iter() + .filter_map(|line| serde_json::from_str(line).ok()) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn record(command: &[&str], invocation: Invocation) -> Record { + let mut record = Record::default(); + record.command = command.iter().map(|c| c.to_string()).collect(); + record.invocation = Some(invocation); + record + } + + #[test] + fn help_version_history_and_sudo_are_not_recorded() { + let run = record(&["styles", "list"], Invocation::Execute); + assert!(recorded(&run, false)); + assert!(!recorded(&run, true), "under sudo"); + assert!(!recorded(&record(&["styles"], Invocation::Help), false)); + assert!(!recorded(&record(&[], Invocation::Version), false)); + assert!(!recorded( + &record(&["history", "list"], Invocation::Execute), + false + )); + } + + #[test] + fn a_line_keeps_the_command_path_and_no_argument() { + let mut run = record(&["search", "forward"], Invocation::Execute); + run.argv = ["search", "forward", "--q", "1600 Pennsylvania Ave"] + .iter() + .map(std::ffi::OsString::from) + .collect(); + let text = serde_json::to_string(&line(&run)).unwrap(); + assert!(text.contains(r#""command":["search","forward"]"#), "{text}"); + assert!(!text.contains("Pennsylvania"), "{text}"); + } +} diff --git a/src/run_record.rs b/src/run_record.rs index 06ec9b6..208dd6e 100644 --- a/src/run_record.rs +++ b/src/run_record.rs @@ -9,7 +9,7 @@ //! chooses field by field what it takes. Best-effort: nothing here can change //! a command's output or exit code. -// Nothing in this tree reads the record yet. +// Some facts are read only by consumers not in this tree yet. #![allow(dead_code)] use std::ffi::OsString; @@ -20,7 +20,7 @@ use clap::parser::ValueSource; use clap::{ArgMatches, Command}; use crate::spec::ServiceSpec; -use crate::{auth, completion, confirm, executor, http, output, tilesets_cli}; +use crate::{auth, completion, confirm, executor, http, output, run_history, tilesets_cli}; const TILESETS: &str = tilesets_cli::COMMAND; @@ -110,6 +110,8 @@ pub(crate) struct Failure { /// Everything the run reported. #[derive(Debug, Default)] pub(crate) struct Record { + /// A random id for this run, set at [`start`]. + pub id: String, /// The command line, without the binary's own path. Raw: tokens are /// still in it. pub argv: Vec, @@ -136,6 +138,7 @@ pub(crate) struct Record { } static RECORD: Mutex = Mutex::new(Record { + id: String::new(), argv: Vec::new(), command: Vec::new(), invocation: None, @@ -169,7 +172,11 @@ fn with_record(f: impl FnOnce(&mut Record)) { pub fn start(argv: &[OsString]) { STARTED.get_or_init(Instant::now); let argv = argv.get(1..).unwrap_or_default().to_vec(); - with_record(|record| record.argv = argv); + let id = uuid_v4(rand::random()); + with_record(|record| { + record.id = id; + record.argv = argv; + }); let previous = std::panic::take_hook(); std::panic::set_hook(Box::new(move |info| { @@ -330,6 +337,21 @@ fn finish_locked(record: &mut Record, exit_code: Option) { record.finished = true; record.duration = STARTED.get().map_or(Duration::ZERO, Instant::elapsed); record.exit_code = exit_code; + run_history::write(record); +} + +fn uuid_v4(mut bytes: [u8; 16]) -> String { + bytes[6] = (bytes[6] & 0x0f) | 0x40; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); + format!( + "{}-{}-{}-{}-{}", + &hex[0..8], + &hex[8..12], + &hex[12..16], + &hex[16..20], + &hex[20..32] + ) } /// The command path, the leaf `Command` and the leaf matches. diff --git a/src/schema.rs b/src/schema.rs index 27c5e57..31a3e8a 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -379,6 +379,14 @@ fn commands(app: &Command, specs: &[ServiceSpec], path: &[String]) -> Vec bool { - match std::env::var_os(MAPBOX_CLI_NO_TELEMETRY_ENV) { - None => true, - Some(value) => { - let value = value.to_string_lossy().trim().to_ascii_lowercase(); - value.is_empty() || NOT_AN_OPT_OUT.contains(&value.as_str()) - } + env_switch(MAPBOX_CLI_NO_TELEMETRY_ENV) != Some(true) +} + +/// A boolean environment variable by this CLI's convention: `None` when +/// unset or empty, `Some(false)` for one of [`NOT_AN_OPT_OUT`], `Some(true)` +/// for anything else. +pub(crate) fn env_switch(name: &str) -> Option { + let value = std::env::var_os(name)?; + let value = value.to_string_lossy().trim().to_ascii_lowercase(); + if value.is_empty() { + None + } else { + Some(!NOT_AN_OPT_OUT.contains(&value.as_str())) } } diff --git a/tests/auth_profiles.rs b/tests/auth_profiles.rs index 2b970dc..8cbdffc 100644 --- a/tests/auth_profiles.rs +++ b/tests/auth_profiles.rs @@ -49,6 +49,7 @@ fn command(home: &Path) -> Command { .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_HISTORY") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", config_dir(home)); @@ -267,24 +268,49 @@ fn listing_existing_profiles_does_not_touch_directory_permissions() { ); } +/// Listing creates no credential store. Command history, on by default, +/// may create the config directory to hold `history/`; with history off, +/// nothing is created at all. #[test] fn an_absent_config_directory_lists_nothing_and_creates_none() { - let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("auth-profiles-absent"); - let _ = std::fs::remove_dir_all(&home); - std::fs::create_dir_all(&home).expect("create the scratch home, with no .mapbox inside it"); - - let json = command(&home) - .args(["-o", "json", "auth", "profiles"]) - .output() - .expect("run mapbox auth profiles -o json"); - assert!( - json.status.success(), - "{}", - String::from_utf8_lossy(&json.stderr) - ); - assert_eq!(stdout(&json), "[]"); - assert!( - !config_dir(&home).exists(), - "listing profiles must not create the config directory" - ); + for (history, allowed) in [("1", &["history"][..]), ("0", &[][..])] { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")) + .join(format!("auth-profiles-absent-{history}")); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(&home).expect("create the scratch home, with no .mapbox inside it"); + + let json = command(&home) + .env("MAPBOX_HISTORY", history) + .args(["-o", "json", "auth", "profiles"]) + .output() + .expect("run mapbox auth profiles -o json"); + assert!( + json.status.success(), + "{}", + String::from_utf8_lossy(&json.stderr) + ); + assert_eq!(stdout(&json), "[]"); + let created: Vec = std::fs::read_dir(config_dir(&home)) + .map(|entries| { + entries + .map(|e| { + e.expect("an entry") + .file_name() + .to_string_lossy() + .into_owned() + }) + .collect() + }) + .unwrap_or_default(); + assert!( + created.iter().all(|name| allowed.contains(&name.as_str())), + "with MAPBOX_HISTORY={history}, listing profiles created {created:?}" + ); + if history == "0" { + assert!( + !config_dir(&home).exists(), + "history off created the directory" + ); + } + } } diff --git a/tests/completion.rs b/tests/completion.rs index 2c5dc1f..72e121d 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -45,6 +45,7 @@ fn command() -> Command { .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_HISTORY") .env("HOME", &home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", home.join(".mapbox")); @@ -354,11 +355,22 @@ fn a_missing_or_unknown_shell_is_a_usage_error() { /// refresh round-trip would be unusable. #[test] fn it_needs_no_token_and_touches_no_credentials() { - let home = sandbox_home(); + // A home of its own: every other test here shares `sandbox_home`, and + // any of them — a usage error, `--schema` — records command history in + // its `.mapbox` while this one is looking. `completion` itself records + // nothing, history on or off. + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("completion-home-untouched"); let config = home.join(".mapbox"); let _ = std::fs::remove_dir_all(&config); + std::fs::create_dir_all(&home).expect("create the home"); - let out = run(&["completion", "zsh"]); + let out = command() + .env("HOME", &home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", &config) + .args(["completion", "zsh"]) + .output() + .expect("run mapbox"); assert!(out.status.success(), "{}", stderr(&out)); assert!( !config.exists(), diff --git a/tests/config.rs b/tests/config.rs index c6e2558..a1ec7c4 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -127,13 +127,16 @@ fn an_unknown_key_or_value_is_a_usage_error_not_a_panic() { fn list_reports_every_setting_including_an_unset_one() { let home = scratch("list"); - // Nothing set yet: list still names the one known key, at its default. + // Nothing set yet: list still names every known key, at its default. let empty = command(&home) .args(["-o", "json", "config", "list"]) .output() .expect("run mapbox config list"); assert!(empty.status.success()); - assert_eq!(stdout(&empty), r#"[{"key":"update-check","value":true}]"#); + assert_eq!( + stdout(&empty), + r#"[{"key":"update-check","value":true},{"key":"history","value":true}]"# + ); let set = command(&home) .args(["config", "set", "update-check", "off"]) @@ -146,14 +149,17 @@ fn list_reports_every_setting_including_an_unset_one() { .output() .expect("run mapbox config list"); assert!(after.status.success()); - assert_eq!(stdout(&after), r#"[{"key":"update-check","value":false}]"#); + assert_eq!( + stdout(&after), + r#"[{"key":"update-check","value":false},{"key":"history","value":true}]"# + ); let text = command(&home) .args(["-o", "text", "config", "list"]) .output() .expect("run mapbox config list"); assert!(text.status.success()); - assert_eq!(stdout(&text), "update-check\toff"); + assert_eq!(stdout(&text), "update-check\toff\nhistory\ton"); } #[test] diff --git a/tests/history.rs b/tests/history.rs new file mode 100644 index 0000000..6babb53 --- /dev/null +++ b/tests/history.rs @@ -0,0 +1,239 @@ +//! End-to-end tests for command history and `mapbox history`. +//! +//! The unit tests in `src/run_history.rs` and `src/history.rs` cover the +//! pure parts — which runs are recorded, what a line keeps, how an id prefix +//! resolves. What they cannot show is what a real run leaves on disk: a line +//! by default, none of what was typed after the command path, nothing at all +//! with history off, and `history` reading back what the runs before it +//! recorded. +//! +//! Nothing here reaches the network. The one request a test makes goes to a +//! proxy on a loopback port nobody is listening on, so it fails at once and +//! is still a request `http::send` saw. + +use std::net::TcpListener; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +use serde_json::Value; + +/// A token-shaped fake. Nothing of it may reach history. +const TOKEN: &str = "pk.eyJ1IjoiZXhhbXBsZS11c2VyIiwiYSI6IngifQ.SIGNATURE-NOT-FOR-HISTORY"; +const SEARCH: &str = "1600 Pennsylvania Ave"; + +fn scratch(name: &str) -> PathBuf { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("history-{name}")); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(&home).expect("create the scratch home"); + home +} + +fn config_dir(home: &Path) -> PathBuf { + home.join(".mapbox") +} + +fn history_dir(home: &Path) -> PathBuf { + config_dir(home).join("history") +} + +fn command(home: &Path) -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + cmd.env_remove("MAPBOX_ACCESS_TOKEN") + .env_remove("MapboxAccessToken") + .env_remove("MAPBOX_USERNAME") + .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_HISTORY") + .env_remove("SUDO_USER") + .env_remove("NO_PROXY") + .env_remove("no_proxy") + .env("MAPBOX_NO_UPDATE_CHECK", "1") + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", config_dir(home)); + cmd +} + +fn run(home: &Path, args: &[&str]) -> Output { + command(home).args(args).output().expect("run mapbox") +} + +/// Every recorded line, oldest first, and the raw bytes they came from. +fn lines(home: &Path) -> (Vec, String) { + let Ok(entries) = std::fs::read_dir(history_dir(home)) else { + return (vec![], String::new()); + }; + let mut files: Vec = entries.map(|e| e.expect("an entry").path()).collect(); + files.sort(); + let raw: String = files + .iter() + .map(|f| std::fs::read_to_string(f).expect("read a history file")) + .collect(); + let parsed = raw + .lines() + .map(|l| serde_json::from_str(l).expect("a history line is JSON")) + .collect(); + (parsed, raw) +} + +/// A request that fails before it leaves the machine: through a proxy on a +/// loopback port with nothing listening. +fn a_refused_search(home: &Path) -> Output { + let port = { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + listener.local_addr().expect("the bound address").port() + }; + command(home) + .env("HTTPS_PROXY", format!("http://127.0.0.1:{port}")) + .args(["search", "forward", "--q", SEARCH, "--token", TOKEN]) + .output() + .expect("run mapbox") +} + +#[test] +fn a_run_is_recorded_by_default_with_its_command_path_only() { + let home = scratch("default"); + let out = a_refused_search(&home); + assert!(!out.status.success(), "the request cannot have succeeded"); + + let (lines, raw) = lines(&home); + assert_eq!(lines.len(), 1, "{raw}"); + let line = &lines[0]; + assert_eq!(line["command"], serde_json::json!(["search", "forward"])); + assert_eq!(line["exitCode"], 1); + assert_eq!(line["requestCount"], 1); + assert!(line["errorCode"].is_string(), "{line}"); + assert!( + line["id"].as_str().is_some_and(|id| id.len() == 36), + "{line}" + ); + + for typed in [SEARCH, "Pennsylvania", TOKEN, "SIGNATURE", "example-user"] { + assert!(!raw.contains(typed), "history kept {typed:?}: {raw}"); + } +} + +#[test] +fn with_history_off_nothing_is_created() { + let home = scratch("off"); + let out = command(&home) + .env("MAPBOX_HISTORY", "0") + .args(["styles", "lsit"]) + .output() + .expect("run mapbox"); + assert!(!out.status.success()); + assert!( + !config_dir(&home).exists(), + "a run with history off created {}", + config_dir(&home).display() + ); +} + +#[test] +fn the_setting_turns_it_off_and_the_variable_overrides_the_setting() { + let home = scratch("switches"); + assert!(run(&home, &["config", "set", "history", "off"]) + .status + .success()); + run(&home, &["styles", "lsit"]); + assert_eq!(lines(&home).0.len(), 0, "off by the setting"); + + command(&home) + .env("MAPBOX_HISTORY", "1") + .args(["styles", "lsit"]) + .output() + .expect("run mapbox"); + assert_eq!( + lines(&home).0.len(), + 1, + "`MAPBOX_HISTORY=1` wins over the setting" + ); +} + +#[test] +fn help_version_completion_history_and_sudo_are_not_recorded() { + let home = scratch("skipped"); + for args in [ + &["--help"][..], + &["--version"], + &["styles", "--help"], + &["completion", "zsh"], + &["history", "list"], + ] { + assert!(run(&home, args).status.success(), "{args:?}"); + } + command(&home) + .env("SUDO_USER", "someone") + .args(["styles", "lsit"]) + .output() + .expect("run mapbox"); + assert_eq!(lines(&home).0.len(), 0, "{}", lines(&home).1); +} + +#[test] +fn history_reads_back_the_runs() { + let home = scratch("read-back"); + a_refused_search(&home); + run(&home, &["styles", "lsit"]); + + let list = run(&home, &["-o", "json", "history", "list"]); + assert!(list.status.success()); + let listed: Value = serde_json::from_slice(&list.stdout).expect("a JSON list"); + let listed = listed.as_array().expect("an array"); + assert_eq!(listed.len(), 2); + assert_eq!( + listed[0]["command"], + serde_json::json!(["styles"]), + "newest first" + ); + let older = listed[1]["id"].as_str().expect("an id"); + + let show = run(&home, &["-o", "json", "history", "show", &older[..8]]); + assert!(show.status.success()); + let shown: Value = serde_json::from_slice(&show.stdout).expect("a JSON run"); + assert_eq!(shown["id"], older); + assert_eq!(shown["command"], serde_json::json!(["search", "forward"])); + + let text = run(&home, &["-o", "text", "history", "list"]); + let text = String::from_utf8_lossy(&text.stdout); + assert!(text.contains("mapbox search forward"), "{text}"); + assert!(!text.contains(SEARCH), "{text}"); + + let missing = run(&home, &["-o", "json", "history", "show", "zzzz"]); + assert!(!missing.status.success()); + assert!( + String::from_utf8_lossy(&missing.stderr).contains(r#""code":"history_not_found""#), + "{}", + String::from_utf8_lossy(&missing.stderr) + ); +} + +#[test] +fn history_with_history_off_says_so_on_stderr() { + let home = scratch("read-off"); + let out = command(&home) + .env("MAPBOX_HISTORY", "0") + .args(["-o", "json", "history", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "[]"); + assert!( + String::from_utf8_lossy(&out.stderr).contains("mapbox config set history on"), + "{}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn days_past_the_thirty_day_window_are_removed() { + let home = scratch("retention"); + std::fs::create_dir_all(history_dir(&home)).expect("the history directory"); + let old = history_dir(&home).join("2000-01-01.jsonl"); + std::fs::write(&old, "{}\n").expect("an old file"); + let not_history = history_dir(&home).join("notes.txt"); + std::fs::write(¬_history, "mine").expect("an unrelated file"); + + run(&home, &["styles", "lsit"]); + assert!(!old.exists(), "a file from 2000 outlived a 30-day window"); + assert!(not_history.exists(), "only dated history files are removed"); +} diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index bdb02f6..11c1036 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -39,6 +39,7 @@ fn command(home: &Path) -> Command { .env_remove("MAPBOX_OUTPUT") .env_remove("MAPBOX_YES") .env_remove("MAPBOX_CONFIG_DIR") + .env_remove("MAPBOX_HISTORY") .env("HOME", home); cmd } @@ -165,20 +166,43 @@ fn the_refusal_carries_a_code_and_a_fix() { } /// The check runs before `config_dir`, which creates the store as a side -/// effect. A CI job that tried to log in should leave nothing behind. +/// effect. A CI job that tried to log in should leave nothing behind but +/// its command history — and with history off, nothing at all. #[test] fn the_refusal_creates_no_credential_directory() { - let home = scratch("no-dir"); - let out = command(&home) - .args(["auth", "login"]) - .output() - .expect("run mapbox"); + for history in ["1", "0"] { + let home = scratch(&format!("no-dir-{history}")); + let out = command(&home) + .env("MAPBOX_HISTORY", history) + .args(["auth", "login"]) + .output() + .expect("run mapbox"); - assert!(!out.status.success()); - assert!( - !home.join(".mapbox").exists(), - "the refusal created the credential store anyway" - ); + assert!(!out.status.success()); + let left: Vec = std::fs::read_dir(home.join(".mapbox")) + .map(|entries| { + entries + .map(|e| { + e.expect("an entry") + .file_name() + .to_string_lossy() + .into_owned() + }) + .collect() + }) + .unwrap_or_default(); + let allowed: &[&str] = if history == "1" { &["history"] } else { &[] }; + assert!( + left.iter().all(|name| allowed.contains(&name.as_str())), + "with MAPBOX_HISTORY={history}, the refusal created {left:?}" + ); + if history == "0" { + assert!( + !home.join(".mapbox").exists(), + "history off created the directory" + ); + } + } } /// One way of saying yes, as a case: a scratch-directory name, the arguments diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 6a7f6b8..ce3b848 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -43,6 +43,8 @@ fn sources() -> Vec<(String, String)> { /// - `agent_skills` — the staging directory it renames skills out of, and the /// skill directory `install --force` replaces. /// - `auth` — `logout`, and the scratch file `write_private` renames from. +/// - `dated_jsonl` — its own dated files past the retention window, matched +/// by exact `YYYY-MM-DD.jsonl` names inside the directory it writes to. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. @@ -50,6 +52,7 @@ fn sources() -> Vec<(String, String)> { const MAY_DELETE: &[&str] = &[ "agent_skills.rs", "auth.rs", + "dated_jsonl.rs", "executor.rs", "generate_skills.rs", "skill_dest.rs", From 1a5e7883874aa5ddead47b7201893c6379da3671 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Mon, 28 Sep 2026 15:07:53 +0300 Subject: [PATCH 2/4] Address review: header for history list, simpler history-off tests - `history list` text output gets a header row. - Tests that hold a run to leaving nothing on disk turn history off in their helper instead of looping over both states; tests/history.rs checks what history leaves, and that skipped runs create nothing. - README says turning history off keeps what was already recorded. - Drop the setting count from config.rs's module doc, which every new key would have to edit. --- README.md | 3 +- docs/commands.md | 3 ++ src/config.rs | 1 - src/history.rs | 31 +++++++++++-------- tests/auth_profiles.rs | 65 +++++++++++++--------------------------- tests/completion.rs | 19 ++++-------- tests/history.rs | 17 +++++++++-- tests/non_interactive.rs | 49 +++++++++--------------------- 8 files changed, 79 insertions(+), 109 deletions(-) diff --git a/README.md b/README.md index c5d3df4..0ada690 100644 --- a/README.md +++ b/README.md @@ -457,7 +457,8 @@ mapbox history show be40d711 # or one run, by any prefix of its id `--help`, `--version`, `completion`, `history` itself and runs under `sudo` are not recorded. `mapbox config set history off` turns history off for good, and `MAPBOX_HISTORY=0` for one shell; with it off, nothing is written -and no directory is created. `MAPBOX_CLI_NO_TELEMETRY` does not affect it. +and no directory is created, but what was already recorded stays until you +delete `~/.mapbox/history`. `MAPBOX_CLI_NO_TELEMETRY` does not affect it. ### Privacy diff --git a/docs/commands.md b/docs/commands.md index aa8f15f..7d382aa 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3415,6 +3415,7 @@ mapbox history list --limit 0 ``` +ID TIME EXIT COMMAND d05b3f4d 2026-09-28T11:20:03.095Z 2 mapbox styles be40d711 2026-09-28T11:20:03.045Z 1 mapbox styles list ``` @@ -3514,6 +3515,8 @@ Requests 1 +--- + ## Doctor ### `mapbox doctor` diff --git a/src/config.rs b/src/config.rs index 09b9cce..a33d479 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,7 +6,6 @@ //! file beside the credentials, written through the same //! [`crate::auth::write_private`] so it gets the same `0600` treatment. //! -//! Two settings — `update-check` and `history` (see [`crate::run_history`]). //! `get`/`set`/`unset` take a `key`, restricted by clap to [`KEYS`], so //! adding a setting is a new key and a new match arm rather than a new //! subcommand. diff --git a/src/history.rs b/src/history.rs index 421a00b..9c5af6b 100644 --- a/src/history.rs +++ b/src/history.rs @@ -63,19 +63,26 @@ pub fn list(matches: &ArgMatches, mode: Mode) -> Result<()> { hint_when_off(); } - let text = entries - .iter() - .map(|entry| { - format!( - "{} {} {:>4} {}", - short_id(entry), - field(entry, "time"), - exit_code(entry), - command_line(entry) - ) - }) + let rows = entries.iter().map(|entry| { + format!( + "{:SHORT_ID$} {:24} {:>4} {}", + short_id(entry), + field(entry, "time"), + exit_code(entry), + command_line(entry) + ) + }); + let text = if entries.is_empty() { + String::new() + } else { + std::iter::once(format!( + "{:SHORT_ID$} {:24} {:>4} COMMAND", + "ID", "TIME", "EXIT" + )) + .chain(rows) .collect::>() - .join("\n"); + .join("\n") + }; let json = entries .iter() .map(|entry| { diff --git a/tests/auth_profiles.rs b/tests/auth_profiles.rs index 8cbdffc..793f7bb 100644 --- a/tests/auth_profiles.rs +++ b/tests/auth_profiles.rs @@ -49,7 +49,9 @@ fn command(home: &Path) -> Command { .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") - .env_remove("MAPBOX_HISTORY") + // Off: these tests hold a run to leaving nothing on disk; what + // history leaves is `tests/history.rs`'s to check. + .env("MAPBOX_HISTORY", "0") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", config_dir(home)); @@ -268,49 +270,24 @@ fn listing_existing_profiles_does_not_touch_directory_permissions() { ); } -/// Listing creates no credential store. Command history, on by default, -/// may create the config directory to hold `history/`; with history off, -/// nothing is created at all. #[test] fn an_absent_config_directory_lists_nothing_and_creates_none() { - for (history, allowed) in [("1", &["history"][..]), ("0", &[][..])] { - let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")) - .join(format!("auth-profiles-absent-{history}")); - let _ = std::fs::remove_dir_all(&home); - std::fs::create_dir_all(&home).expect("create the scratch home, with no .mapbox inside it"); - - let json = command(&home) - .env("MAPBOX_HISTORY", history) - .args(["-o", "json", "auth", "profiles"]) - .output() - .expect("run mapbox auth profiles -o json"); - assert!( - json.status.success(), - "{}", - String::from_utf8_lossy(&json.stderr) - ); - assert_eq!(stdout(&json), "[]"); - let created: Vec = std::fs::read_dir(config_dir(&home)) - .map(|entries| { - entries - .map(|e| { - e.expect("an entry") - .file_name() - .to_string_lossy() - .into_owned() - }) - .collect() - }) - .unwrap_or_default(); - assert!( - created.iter().all(|name| allowed.contains(&name.as_str())), - "with MAPBOX_HISTORY={history}, listing profiles created {created:?}" - ); - if history == "0" { - assert!( - !config_dir(&home).exists(), - "history off created the directory" - ); - } - } + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("auth-profiles-absent"); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(&home).expect("create the scratch home, with no .mapbox inside it"); + + let json = command(&home) + .args(["-o", "json", "auth", "profiles"]) + .output() + .expect("run mapbox auth profiles -o json"); + assert!( + json.status.success(), + "{}", + String::from_utf8_lossy(&json.stderr) + ); + assert_eq!(stdout(&json), "[]"); + assert!( + !config_dir(&home).exists(), + "listing profiles must not create the config directory" + ); } diff --git a/tests/completion.rs b/tests/completion.rs index 72e121d..904fb17 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -45,7 +45,9 @@ fn command() -> Command { .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") - .env_remove("MAPBOX_HISTORY") + // Off: these tests hold a run to leaving nothing on disk; what + // history leaves is `tests/history.rs`'s to check. + .env("MAPBOX_HISTORY", "0") .env("HOME", &home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", home.join(".mapbox")); @@ -355,22 +357,11 @@ fn a_missing_or_unknown_shell_is_a_usage_error() { /// refresh round-trip would be unusable. #[test] fn it_needs_no_token_and_touches_no_credentials() { - // A home of its own: every other test here shares `sandbox_home`, and - // any of them — a usage error, `--schema` — records command history in - // its `.mapbox` while this one is looking. `completion` itself records - // nothing, history on or off. - let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("completion-home-untouched"); + let home = sandbox_home(); let config = home.join(".mapbox"); let _ = std::fs::remove_dir_all(&config); - std::fs::create_dir_all(&home).expect("create the home"); - let out = command() - .env("HOME", &home) - .env("XDG_CONFIG_HOME", home.join(".config")) - .env("MAPBOX_CONFIG_DIR", &config) - .args(["completion", "zsh"]) - .output() - .expect("run mapbox"); + let out = run(&["completion", "zsh"]); assert!(out.status.success(), "{}", stderr(&out)); assert!( !config.exists(), diff --git a/tests/history.rs b/tests/history.rs index 6babb53..99aad4c 100644 --- a/tests/history.rs +++ b/tests/history.rs @@ -113,7 +113,15 @@ fn a_run_is_recorded_by_default_with_its_command_path_only() { } #[test] -fn with_history_off_nothing_is_created() { +fn a_run_leaves_only_history_and_with_it_off_nothing() { + let home = scratch("on"); + run(&home, &["styles", "lsit"]); + let created: Vec<_> = std::fs::read_dir(config_dir(&home)) + .expect("the config directory") + .map(|e| e.expect("an entry").file_name()) + .collect(); + assert_eq!(created, ["history"]); + let home = scratch("off"); let out = command(&home) .env("MAPBOX_HISTORY", "0") @@ -166,7 +174,11 @@ fn help_version_completion_history_and_sudo_are_not_recorded() { .args(["styles", "lsit"]) .output() .expect("run mapbox"); - assert_eq!(lines(&home).0.len(), 0, "{}", lines(&home).1); + assert!( + !config_dir(&home).exists(), + "a run history skips created {}", + config_dir(&home).display() + ); } #[test] @@ -195,6 +207,7 @@ fn history_reads_back_the_runs() { let text = run(&home, &["-o", "text", "history", "list"]); let text = String::from_utf8_lossy(&text.stdout); + assert!(text.starts_with("ID "), "a header row first: {text}"); assert!(text.contains("mapbox search forward"), "{text}"); assert!(!text.contains(SEARCH), "{text}"); diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index 11c1036..63e3a13 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -37,9 +37,11 @@ fn command(home: &Path) -> Command { .env_remove("MapboxAccessToken") .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") + // Off: these tests hold a run to leaving nothing on disk; what + // history leaves is `tests/history.rs`'s to check. + .env("MAPBOX_HISTORY", "0") .env_remove("MAPBOX_YES") .env_remove("MAPBOX_CONFIG_DIR") - .env_remove("MAPBOX_HISTORY") .env("HOME", home); cmd } @@ -166,43 +168,20 @@ fn the_refusal_carries_a_code_and_a_fix() { } /// The check runs before `config_dir`, which creates the store as a side -/// effect. A CI job that tried to log in should leave nothing behind but -/// its command history — and with history off, nothing at all. +/// effect. A CI job that tried to log in should leave nothing behind. #[test] fn the_refusal_creates_no_credential_directory() { - for history in ["1", "0"] { - let home = scratch(&format!("no-dir-{history}")); - let out = command(&home) - .env("MAPBOX_HISTORY", history) - .args(["auth", "login"]) - .output() - .expect("run mapbox"); + let home = scratch("no-dir"); + let out = command(&home) + .args(["auth", "login"]) + .output() + .expect("run mapbox"); - assert!(!out.status.success()); - let left: Vec = std::fs::read_dir(home.join(".mapbox")) - .map(|entries| { - entries - .map(|e| { - e.expect("an entry") - .file_name() - .to_string_lossy() - .into_owned() - }) - .collect() - }) - .unwrap_or_default(); - let allowed: &[&str] = if history == "1" { &["history"] } else { &[] }; - assert!( - left.iter().all(|name| allowed.contains(&name.as_str())), - "with MAPBOX_HISTORY={history}, the refusal created {left:?}" - ); - if history == "0" { - assert!( - !home.join(".mapbox").exists(), - "history off created the directory" - ); - } - } + assert!(!out.status.success()); + assert!( + !home.join(".mapbox").exists(), + "the refusal created the credential store anyway" + ); } /// One way of saying yes, as a case: a scratch-directory name, the arguments From 2037c9280ddfff5c9f700b826c50526846ad2118 Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Mon, 28 Sep 2026 15:13:45 +0300 Subject: [PATCH 3/4] Hold command history to 10 MB Moves dated_jsonl::shed here from the diagnostic-log PR so history, on by default, is bounded by size as well as by age. trim now keeps exactly the bytes shed hands it; the margin was applied twice before. --- CHANGELOG.md | 2 +- README.md | 3 +- docs/commands.md | 2 +- src/dated_jsonl.rs | 160 ++++++++++++++++++++++++++++++++++++++--- src/run_history.rs | 7 +- tests/source_guards.rs | 6 +- 6 files changed, 165 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5448635..a3b4e7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ that may never merge. They are not releases and are not listed here. ## Unreleased - Command history, on by default: each run appends one line to - `~/.mapbox/history/.jsonl`, kept 30 days, with its command path, exit + `~/.mapbox/history/.jsonl`, kept 30 days and at most 10 MB, with its command path, exit code, error code, duration and request ids — never an argument value. It stays on your machine. `mapbox history list` and `mapbox history show` read it back. Turn it off with `mapbox config set history off` or diff --git a/README.md b/README.md index 0ada690..c5ade6f 100644 --- a/README.md +++ b/README.md @@ -442,7 +442,8 @@ environment variable happens to be set in. ### Command history Each run appends one line to `~/.mapbox/history/.jsonl` (or under -`$MAPBOX_CONFIG_DIR`), kept for 30 days: which command ran (its command path, +`$MAPBOX_CONFIG_DIR`), kept for 30 days and at most 10 MB, oldest dropped +first: which command ran (its command path, like `search forward`), how it ended, how long it took and the request ids support can look up. Argument values are never recorded — not what you searched for, not a file path, not a token. The files are readable only by diff --git a/docs/commands.md b/docs/commands.md index 7d382aa..cbeb1bd 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3379,7 +3379,7 @@ update-check cleared, now on (default). ## History The runs [command history](../README.md#command-history) recorded on this -machine over the last 30 days: which command ran, how it ended, how long it +machine over the last 30 days, up to 10 MB: which command ran, how it ended, how long it took and the request ids support can look up. Argument values are never recorded, so a run shows as its command path — `mapbox search forward`, not what was searched for. History is on by default; with it off diff --git a/src/dated_jsonl.rs b/src/dated_jsonl.rs index 31375f4..098cc34 100644 --- a/src/dated_jsonl.rs +++ b/src/dated_jsonl.rs @@ -1,8 +1,9 @@ //! Private, append-only, one-file-per-UTC-day JSONL directories under the -//! config directory, pruned to a fixed number of days, for the consumers of -//! [`crate::run_record`] that keep records on disk. The only files this -//! deletes are ones named exactly `YYYY-MM-DD.jsonl` inside the directory -//! it was handed. +//! config directory, pruned to a fixed number of days and held to a total +//! size, for the consumers of [`crate::run_record`] that keep records on +//! disk. The only files this deletes or replaces are ones named exactly +//! `YYYY-MM-DD.jsonl` inside the directory it was handed, and the scratch +//! file a trim writes beside one. use std::io::Write; use std::path::{Path, PathBuf}; @@ -56,17 +57,51 @@ pub(crate) fn append(dir: &Path, line: &str, keep_days: u64) { } } -/// Every line in `dir`'s dated files, oldest first. A missing directory is -/// no lines. -pub(crate) fn read_all(dir: &Path) -> Vec { +/// Holds `dir`'s dated files, together, to `limit` bytes by dropping the +/// oldest lines first — whole days while a day is all that has to go, then +/// the oldest lines of the oldest day left. Sheds down to nine tenths of +/// `limit`, so the next run does not have to shed again. +pub(crate) fn shed(dir: &Path, limit: u64) { + let mut files: Vec<(String, u64)> = dated_names(dir) + .into_iter() + .filter_map(|name| Some((name.clone(), std::fs::metadata(dir.join(&name)).ok()?.len()))) + .collect(); + let total: u64 = files.iter().map(|(_, size)| size).sum(); + if total <= limit { + return; + } + let mut excess = total - limit / 10 * 9; + files.sort(); + for (name, size) in files { + if excess == 0 { + break; + } + let path = dir.join(&name); + if size <= excess { + let _ = std::fs::remove_file(&path); + excess -= size; + } else { + trim(&path, size - excess); + excess = 0; + } + } +} + +fn dated_names(dir: &Path) -> Vec { let Ok(entries) = std::fs::read_dir(dir) else { return vec![]; }; - let mut names: Vec = entries + entries .flatten() .map(|entry| entry.file_name().to_string_lossy().into_owned()) .filter(|name| dated_file(name).is_some()) - .collect(); + .collect() +} + +/// Every line in `dir`'s dated files, oldest first. A missing directory is +/// no lines. +pub(crate) fn read_all(dir: &Path) -> Vec { + let mut names = dated_names(dir); names.sort(); names .iter() @@ -92,6 +127,69 @@ fn open_private(path: &Path) -> std::io::Result { options.open(path) } +/// Keeps the newest whole lines of `path` that fit in `limit` bytes. +/// +/// Written to a scratch file and renamed over the original. A parallel run +/// that appends between the read and the rename loses its line: these files +/// are best-effort, and a lock would make every run pay for a rare race. +fn trim(path: &Path, limit: u64) { + let Ok(text) = std::fs::read_to_string(path) else { + return; + }; + let kept = newest_lines(&text, limit as usize); + if kept.is_empty() { + let _ = std::fs::remove_file(path); + return; + } + let Some(name) = path.file_name() else { + return; + }; + let scratch = path.with_file_name(format!( + ".{}.trim-{}", + name.to_string_lossy(), + std::process::id() + )); + let written = create_private(&scratch) + .and_then(|mut file| file.write_all(kept.as_bytes())) + .and_then(|()| std::fs::rename(&scratch, path)); + if written.is_err() { + let _ = std::fs::remove_file(&scratch); + } +} + +/// The longest suffix of `text` made of whole lines and no longer than +/// `target` bytes. +fn newest_lines(text: &str, target: usize) -> &str { + if text.len() <= target { + return text; + } + let from = text.len() - target; + let bytes = text.as_bytes(); + // The first line that starts at or after `from`. Always just past a + // `\n`, so never inside a character. + let start = if bytes[from - 1] == b'\n' { + from + } else { + bytes[from..] + .iter() + .position(|&b| b == b'\n') + .map_or(text.len(), |p| from + p + 1) + }; + &text[start..] +} + +/// Creates `path` `0600`, failing if it exists. +fn create_private(path: &Path) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + fn prune(dir: &Path, now: u64, keep_days: u64) { let (oldest_kept, _) = utc_date(now.saturating_sub(keep_days.saturating_sub(1) * 86_400)); let Ok(entries) = std::fs::read_dir(dir) else { @@ -193,4 +291,48 @@ mod tests { ] ); } + + #[test] + fn a_trim_keeps_the_newest_whole_lines() { + let text = "aaaa\nbbbb\ncccc\n"; + assert_eq!(newest_lines(text, 100), text); + assert_eq!(newest_lines(text, 10), "bbbb\ncccc\n"); + assert_eq!(newest_lines(text, 9), "cccc\n"); + assert_eq!(newest_lines(text, 4), ""); + } + + #[test] + fn shedding_drops_the_oldest_days_then_the_oldest_lines() { + let dir = std::env::temp_dir().join(format!("mapbox-dated-shed-{}", rand::random::())); + std::fs::create_dir_all(&dir).unwrap(); + let day = |n: u32| format!("2026-09-{n:02}.jsonl"); + // Three days of ten 100-byte lines each. + for n in 1..=3 { + let text: String = (0..10).map(|i| format!("{n}-{i:<96}\n")).collect(); + std::fs::write(dir.join(day(n)), text).unwrap(); + } + let mine = dir.join("notes.txt"); + std::fs::write(&mine, "x".repeat(5000)).unwrap(); + + shed(&dir, 2000); + let left = read_all(&dir); + let total: usize = left.iter().map(|l| l.len() + 1).sum(); + let mine_kept = mine.exists(); + std::fs::remove_dir_all(&dir).unwrap(); + + assert!(mine_kept, "only dated files are shed"); + assert!(total <= 1800, "{total}"); + assert!( + left.iter().all(|l| !l.starts_with("1-")), + "the oldest day went first" + ); + assert!( + left.iter().any(|l| l.starts_with("2-")), + "only as much as needed" + ); + assert!( + left.last().unwrap().starts_with("3-9"), + "the newest line stays" + ); + } } diff --git a/src/run_history.rs b/src/run_history.rs index 088da10..5fa944d 100644 --- a/src/run_history.rs +++ b/src/run_history.rs @@ -1,6 +1,7 @@ //! Command history: one line of execution metadata per run in //! `~/.mapbox/history/.jsonl` (or under `$MAPBOX_CONFIG_DIR`), -//! kept for [`RETENTION_DAYS`] days and read back by `mapbox history`. +//! kept for [`RETENTION_DAYS`] days and at most [`LIMIT_BYTES`], oldest +//! first, and read back by `mapbox history`. //! //! On by default, so it keeps only what is safe to keep without anyone //! having asked: the command path from the command tree (`search forward`, @@ -33,6 +34,9 @@ use crate::{auth, config, dated_jsonl, history, telemetry}; const DIR: &str = "history"; const HISTORY_ENV: &str = "MAPBOX_HISTORY"; pub(crate) const RETENTION_DAYS: u64 = 30; +/// Tens of thousands of runs: a script calling this in a loop must not fill +/// the disk before thirty days are up. +const LIMIT_BYTES: u64 = 10 * 1024 * 1024; /// The last few are enough to hand to support; a paginated run can make /// hundreds of requests. const MAX_REQUEST_IDS: usize = 5; @@ -73,6 +77,7 @@ pub(crate) fn write(record: &Record) { }; if let Some(dir) = dated_jsonl::private_dir(DIR) { dated_jsonl::append(&dir, &text, RETENTION_DAYS); + dated_jsonl::shed(&dir, LIMIT_BYTES); } } diff --git a/tests/source_guards.rs b/tests/source_guards.rs index ce3b848..3526c4c 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -43,8 +43,10 @@ fn sources() -> Vec<(String, String)> { /// - `agent_skills` — the staging directory it renames skills out of, and the /// skill directory `install --force` replaces. /// - `auth` — `logout`, and the scratch file `write_private` renames from. -/// - `dated_jsonl` — its own dated files past the retention window, matched -/// by exact `YYYY-MM-DD.jsonl` names inside the directory it writes to. +/// - `dated_jsonl` — its own dated files past the retention window or the +/// size limit, matched by exact `YYYY-MM-DD.jsonl` names inside the +/// directory it writes to, and the scratch file a trim leaves when its +/// rename fails. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. From b1ad1244d3b4b15e73fb92b3c79e9e203322ac8f Mon Sep 17 00:00:00 2001 From: Mofei Zhu Date: Mon, 28 Sep 2026 12:21:36 +0300 Subject: [PATCH 4/4] Record one cli.command telemetry event per run Built from the run_record the foundation change already collects, so no call site changes here: telemetry_event chooses each field from the record under its privacy rules, and telemetry_sink delivers the line (to a local file for now, through dated_jsonl). run_record::finish hands the record to telemetry. --- CHANGELOG.md | 7 + README.md | 6 + src/main.rs | 2 + src/run_record.rs | 9 +- src/telemetry.rs | 15 + src/telemetry_event.rs | 872 ++++++++++++++++++++++++++++++++++++++ src/telemetry_sink.rs | 138 ++++++ tests/history.rs | 2 + tests/source_guards.rs | 2 + tests/telemetry_events.rs | 272 ++++++++++++ 10 files changed, 1322 insertions(+), 3 deletions(-) create mode 100644 src/telemetry_event.rs create mode 100644 src/telemetry_sink.rs create mode 100644 tests/telemetry_events.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a3b4e7a..1f4b0fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ that may never merge. They are not releases and are not listed here. code; it does find a `~/.mapbox/history` directory it didn't before, and `mapbox config list` now reports a second key, `history`. +- Each run records one `cli.command` telemetry event, appended to + `~/.mapbox/.telemetry/.jsonl` and kept for 7 days. Nothing is sent + anywhere by default. It never touches stdout, the exit code or how long a + command takes, so a script sees no difference; a CI job does find a + `.mapbox/.telemetry` directory it didn't before. `MAPBOX_CLI_NO_TELEMETRY=1` + turns it off. + - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API parameter this CLI's specs don't declare a flag for. diff --git a/README.md b/README.md index c5ade6f..9ad16d2 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,12 @@ Every request sends `User-Agent: mapbox-cli/` and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` keeps even future markers out of that header. +Each run also appends one event to `~/.mapbox/.telemetry/.jsonl` +(kept for 7 days): the command's name, its options (a value only when it +comes from a fixed list, otherwise just its length or size), how it ended, +and how long it took — never a token, a file path or free text you typed. +It stays on this machine. `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. + ### Agent skills ```sh diff --git a/src/main.rs b/src/main.rs index 52319c7..ab08fcb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -35,6 +35,8 @@ mod schema; mod skill_dest; mod spec; mod telemetry; +mod telemetry_event; +mod telemetry_sink; mod tilesets_cli; mod uninstall; mod update_check; diff --git a/src/run_record.rs b/src/run_record.rs index 208dd6e..5051b68 100644 --- a/src/run_record.rs +++ b/src/run_record.rs @@ -20,7 +20,9 @@ use clap::parser::ValueSource; use clap::{ArgMatches, Command}; use crate::spec::ServiceSpec; -use crate::{auth, completion, confirm, executor, http, output, run_history, tilesets_cli}; +use crate::{ + auth, completion, confirm, executor, http, output, run_history, telemetry_event, tilesets_cli, +}; const TILESETS: &str = tilesets_cli::COMMAND; @@ -338,9 +340,10 @@ fn finish_locked(record: &mut Record, exit_code: Option) { record.duration = STARTED.get().map_or(Duration::ZERO, Instant::elapsed); record.exit_code = exit_code; run_history::write(record); + telemetry_event::deliver(record); } -fn uuid_v4(mut bytes: [u8; 16]) -> String { +pub(crate) fn uuid_v4(mut bytes: [u8; 16]) -> String { bytes[6] = (bytes[6] & 0x0f) | 0x40; bytes[8] = (bytes[8] & 0x3f) | 0x80; let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); @@ -389,7 +392,7 @@ fn command_from_argv(app: &Command, argv: &[OsString]) -> Vec { /// Subcommand names from `words`, in order, for as long as each word names /// a subcommand of the one before. Flags and their values are skipped; the /// first word that is neither ends the walk. -fn tree_path(app: &Command, words: impl IntoIterator) -> Vec { +pub(crate) fn tree_path(app: &Command, words: impl IntoIterator) -> Vec { let mut path = vec![]; let mut command = app; for word in words { diff --git a/src/telemetry.rs b/src/telemetry.rs index 1396369..afe4176 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -58,6 +58,21 @@ fn arch_marker() -> String { format!("arch/{}", spelled_arch(std::env::consts::ARCH)) } +/// The CPU architecture as the event's `env.arch` spells it. +pub(crate) fn arch() -> &'static str { + spelled_arch(std::env::consts::ARCH) +} + +/// Whether this run is in CI, by the same rule as the `env/ci` marker. +pub(crate) fn in_ci() -> bool { + ci_marker().is_some() +} + +/// For `crate::telemetry_event`, which may not reach for stdout itself. +pub(crate) fn stdout_is_terminal() -> bool { + std::io::stdout().is_terminal() +} + fn spelled_arch(arch: &str) -> &str { match arch { "aarch64" => "arm64", diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs new file mode 100644 index 0000000..3828d12 --- /dev/null +++ b/src/telemetry_event.rs @@ -0,0 +1,872 @@ +//! One `cli.command` event per run: what ran and how it ended. +//! +//! Built from the [`crate::run_record::Record`] at the end of each run and +//! handed to [`crate::telemetry_sink`], which decides where it is delivered; +//! this module decides only what it contains. The record holds raw facts — +//! the command line, URLs, error messages — and every field of the event is +//! chosen from it here, explicitly, so this file is the whole of what can +//! leave the machine. +//! +//! What this refuses to send is the point of it. Argument values leave +//! only when they come from a fixed set (an enum, a boolean, a number the +//! spec types, an allowlisted code); a free string is sent as its length, a +//! file as its size, a coordinate as its name alone. Command names come from +//! the command tree, never from argv. A token is read for its prefix and its +//! account claim and nothing else. +//! +//! Best-effort throughout: nothing here can change a command's output, its +//! exit code, or how long it takes to return. With telemetry off +//! (`MAPBOX_CLI_NO_TELEMETRY`), nothing is built or written. + +use std::io::IsTerminal; +use std::path::Path; +use std::sync::{Mutex, OnceLock}; +use std::time::{Duration, SystemTime}; + +use clap::Command; +use serde::Serialize; + +use crate::run_record::{self, uuid_v4, Invocation, Record}; +use crate::{agent_detect, confirm, executor, http, output, schema, telemetry, telemetry_sink}; + +const EVENT: &str = "cli.command"; +const SCHEMA_VERSION: &str = "2.0"; +const SDK_IDENTIFIER: &str = "mapbox-cli"; +const CURRENT: &str = env!("CARGO_PKG_VERSION"); + +/// The largest `--data @` file read for its top-level keys; above +/// it, only the size is recorded. +const MAX_DATA_TO_PARSE: u64 = 256 * 1024; + +/// Set by a workflow on each step it launches, to its own [`event_id`], so +/// a `mapbox` run inside a workflow step records which run started it. +const PARENT_EVENT_ENV: &str = "MAPBOX_CLI_PARENT_EVENT"; + +const USER_ID_FILE: &str = "user-id"; +const LAST_VERSION_FILE: &str = "last-version"; + +// Bounds from the schema. An event over any of them is rejected whole at +// ingest, so they are enforced here rather than trusted. +const MAX_PARAMS: usize = 50; +const MAX_VALUE: usize = 200; +const MAX_NAME: usize = 64; +const MAX_KEYS: usize = 50; +const MAX_COMMAND_LEVELS: usize = 8; +const MAX_COMMAND_NAME: usize = 32; +const MAX_REQUEST_IDS: usize = 5; +const MAX_REQUEST_ID: usize = 128; +const MAX_CODE: usize = 64; +const MAX_STEPS: usize = 20; +const MAX_VERSION: usize = 32; + +/// Options that are top-level fields or `invocation`, so never `params`. +const NOT_PARAMS: &[&str] = &[ + "token", + "profile", + "use-login", + "debug", + confirm::ARG, + http::TIMEOUT_ARG, + output::ARG, + schema::ARG, + executor::DRY_RUN_ARG, + "help", + "version", +]; + +/// Global options with no top-level field, sent as free strings. They are +/// read from the root matches: a leaf `Command` does not list the globals +/// it inherits. +const GLOBAL_PARAMS: &[&str] = &["username", output::FILTER_ARG]; + +/// Free strings whose values are codes, not user data. +const ALLOWLISTED: &[&str] = &["language", "country", "types"]; + +/// Numbers that are sent by name only: together they are a location. +const COORDINATES: &[&str] = &["lon", "lat", "longitude", "latitude"]; + +/// The Python `tilesets` CLI's own commands (`mapbox_tilesets/scripts/cli.py`). +/// A forwarded first word outside this list is recorded as `other`, since it +/// is whatever the user typed. +const TILESETS_COMMANDS: &[&str] = &[ + "add-source", + "create", + "delete", + "delete-changeset", + "delete-source", + "estimate-area", + "job", + "jobs", + "list", + "list-activity", + "list-sources", + "publish", + "publish-changesets", + "status", + "tilejson", + "update", + "update-recipe", + "upload-changeset", + "upload-raster-source", + "upload-source", + "validate-recipe", + "validate-source", + "view-changeset", + "view-recipe", + "view-source", +]; + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +pub(crate) struct Param { + name: String, + #[serde(skip_serializing_if = "Option::is_none")] + value: Option, + #[serde(skip_serializing_if = "Option::is_none")] + length: Option, + #[serde(skip_serializing_if = "Option::is_none")] + bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + keys: Option>, +} + +/// Where a workflow came from. Only Mapbox names its `Builtin` and +/// `Marketplace` workflows; a `Custom` one is named by the user, so its name +/// is never recorded. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] // Used by the workflow commands, which don't exist yet. +pub(crate) enum WorkflowSource { + Builtin, + Marketplace, + Custom, +} + +impl WorkflowSource { + fn as_str(self) -> &'static str { + match self { + WorkflowSource::Builtin => "builtin", + WorkflowSource::Marketplace => "marketplace", + WorkflowSource::Custom => "custom", + } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Workflow { + source: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + step_count: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + steps: Vec, +} + +/// One step of a workflow run. Built only by [`cli_step`] and +/// [`script_step`], so a script step can never carry a command name or an +/// error category: nothing about a user's script is recorded beyond how it +/// exited and how long it took. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Step { + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + duration_ms: u64, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +struct Auth { + source: &'static str, + #[serde(rename = "type")] + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + account: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Network { + request_count: u32, + #[serde(skip_serializing_if = "Option::is_none")] + status: Option, + #[serde(skip_serializing_if = "Option::is_none")] + response_bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + request_body_bytes: Option, + network_ms: u64, + #[serde(skip_serializing_if = "Vec::is_empty")] + request_ids: Vec, + #[serde(skip_serializing_if = "std::ops::Not::not")] + more_pages: bool, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Cli { + #[serde(skip_serializing_if = "Option::is_none")] + build_channel: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + build_id: Option<&'static str>, + install_method: &'static str, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Env { + arch: &'static str, + ci: bool, + #[serde(skip_serializing_if = "Option::is_none")] + agent: Option<&'static str>, + stdin_tty: bool, + stdout_tty: bool, +} + +/// The event as sent. Field order is the schema's. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Event { + event: &'static str, + version: &'static str, + created: String, + event_id: String, + #[serde(skip_serializing_if = "Option::is_none")] + parent_event_id: Option, + user_id: String, + sdk_identifier: &'static str, + sdk_version: &'static str, + operating_system: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + invocation: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + workflow: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + params: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + usage_error: Option, + #[serde(skip_serializing_if = "Option::is_none")] + output: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + output_source: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + dry_run: Option, + #[serde(skip_serializing_if = "Option::is_none")] + debug: Option, + #[serde(skip_serializing_if = "Option::is_none")] + yes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + profile: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + timeout_seconds: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auth: Option, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + stdout_bytes: u64, + duration_ms: u64, + #[serde(skip_serializing_if = "Option::is_none")] + auth_step: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + update_notice: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_version: Option, + #[serde(skip_serializing_if = "Option::is_none")] + network: Option, + cli: Cli, + env: Env, +} + +/// The workflow this run belongs to. Kept here rather than in the record: +/// the workflow commands that report it don't exist yet. +static WORKFLOW: Mutex> = Mutex::new(None); +static EVENT_ID: OnceLock = OnceLock::new(); + +fn with_workflow(f: impl FnOnce(&mut Option)) { + if let Ok(mut workflow) = WORKFLOW.lock() { + f(&mut workflow); + } +} + +/// Builds the run's event and hands it to the sink, unless telemetry is off. +pub(crate) fn deliver(record: &Record) { + if !telemetry::telemetry_allowed() { + return; + } + let event = build(record); + if let Ok(line) = serde_json::to_string(&event) { + telemetry_sink::deliver(&line); + } +} + +fn build(record: &Record) -> Event { + let options = record.options.as_ref(); + Event { + event: EVENT, + version: SCHEMA_VERSION, + created: timestamp(SystemTime::now()), + event_id: event_id().to_string(), + parent_event_id: parent_event_id(), + user_id: user_id(), + sdk_identifier: SDK_IDENTIFIER, + sdk_version: CURRENT, + operating_system: std::env::consts::OS, + command: command_field(record), + invocation: record.invocation.map(Invocation::as_str), + workflow: WORKFLOW.lock().ok().and_then(|workflow| workflow.clone()), + params: params(record), + usage_error: record + .usage_error + .as_deref() + .map(|kind| clip(kind, MAX_CODE)), + output: options.map(|o| o.output), + output_source: options.map(|o| o.output_source), + dry_run: options.map(|o| o.dry_run), + debug: options.map(|o| o.debug), + yes: options.map(|o| o.yes), + profile: options.map(|o| match o.profile.as_deref() { + None | Some("default") => "default", + Some(_) => "named", + }), + timeout_seconds: options.and_then(|o| o.timeout).map(|t| t.as_secs_f64()), + auth: record.token.as_ref().map(|token| Auth { + source: token.source.as_str(), + kind: token.kind, + account: token.account.as_deref().map(|a| clip(a, MAX_NAME)), + }), + exit_code: record.exit_code, + error_code: record.error.as_ref().map(|e| clip(&e.code, MAX_CODE)), + stdout_bytes: record.stdout_bytes, + duration_ms: record.duration.as_millis() as u64, + auth_step: record.auth_step, + update_notice: record + .update_notice + .as_deref() + .map(|v| clip(v, MAX_VERSION)), + previous_version: previous_version(), + network: network(record), + cli: Cli { + build_channel: option_env!("MAPBOX_CLI_BUILD_ENV"), + build_id: option_env!("MAPBOX_CLI_BUILD_ID"), + install_method: install_method(std::env::current_exe().ok().as_deref()), + }, + env: Env { + arch: telemetry::arch(), + ci: telemetry::in_ci(), + agent: agent_detect::detect_agent(), + stdin_tty: std::io::stdin().is_terminal(), + stdout_tty: telemetry::stdout_is_terminal(), + }, + } +} + +const TILESETS: &str = crate::tilesets_cli::COMMAND; + +fn command_field(record: &Record) -> Option> { + let first = record.command.first()?; + if first == TILESETS { + let mut command = vec![TILESETS.to_string()]; + if let Some(word) = &record.tilesets_word { + command.push(tilesets_word(word).to_string()); + } + return Some(command); + } + Some(clip_command(record.command.clone())) +} + +/// Arguments of an executed command, as the schema allows them: the leaf's +/// own, less [`NOT_PARAMS`], then the [`GLOBAL_PARAMS`]. `tilesets-cli`'s +/// forwarded words are not arguments of this CLI and are never sent. +fn params(record: &Record) -> Vec { + if record.invocation != Some(Invocation::Execute) { + return vec![]; + } + let leaf = record + .args + .iter() + .filter(|arg| !arg.global && !NOT_PARAMS.contains(&arg.id.as_str())); + let global = record + .args + .iter() + .filter(|arg| arg.global && GLOBAL_PARAMS.contains(&arg.id.as_str())); + leaf.chain(global) + .take(MAX_PARAMS) + .map(|arg| { + classify( + &arg.name, + &arg.values, + arg.takes_values, + arg.enumerated, + arg.numeric, + ) + }) + .collect() +} + +fn network(record: &Record) -> Option { + if record.requests.is_empty() && !record.more_pages { + return None; + } + let mut network = Network { + more_pages: record.more_pages, + ..Network::default() + }; + for request in &record.requests { + network.request_count = network.request_count.saturating_add(1); + network.status = request.status; + network.response_bytes = sum(network.response_bytes, request.response_bytes); + network.request_body_bytes = sum(network.request_body_bytes, request.request_body_bytes); + network.network_ms = network + .network_ms + .saturating_add(request.elapsed.as_millis() as u64); + if let Some(id) = &request.request_id { + if network.request_ids.len() < MAX_REQUEST_IDS { + network.request_ids.push(clip(id, MAX_REQUEST_ID)); + } + } + } + Some(network) +} + +/// The workflow this run adds, removes or runs. `name` is dropped for a +/// `Custom` workflow, whatever the caller passes. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow(source: WorkflowSource, name: Option<&str>) { + with_workflow(|workflow| *workflow = Some(workflow_field(source, name))); +} + +fn workflow_field(source: WorkflowSource, name: Option<&str>) -> Workflow { + let name = match source { + WorkflowSource::Custom => None, + _ => name.map(|name| clip(name, MAX_NAME)), + }; + Workflow { + source: source.as_str(), + name, + step_count: None, + steps: Vec::new(), + } +} + +/// How many steps the running workflow has, which can be more than the +/// [`MAX_STEPS`] recorded. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow_step_count(count: usize) { + with_workflow(|workflow| { + if let Some(workflow) = workflow.as_mut() { + workflow.step_count = Some(u32::try_from(count).unwrap_or(u32::MAX)); + } + }); +} + +/// A step that ran a `mapbox` command. `words` is the step's command line; +/// only the names the command tree has are kept, so an argument can't pass +/// for a command name. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) { + let step = cli_step(app, words, exit_code, error_code, duration); + with_workflow(|workflow| push_step(workflow, step)); +} + +/// A step that ran one of the user's own scripts. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_script_step(exit_code: Option, duration: Duration) { + let step = script_step(exit_code, duration); + with_workflow(|workflow| push_step(workflow, step)); +} + +fn cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) -> Step { + let path = run_record::tree_path(app, words.iter().map(|word| word.to_string())); + Step { + kind: "cli", + command: (!path.is_empty()).then(|| clip_command(path)), + exit_code, + error_code: error_code.map(|code| clip(code, MAX_CODE)), + duration_ms: duration.as_millis() as u64, + } +} + +fn script_step(exit_code: Option, duration: Duration) -> Step { + Step { + kind: "script", + command: None, + exit_code, + error_code: None, + duration_ms: duration.as_millis() as u64, + } +} + +/// Steps belong to a workflow: one reported before [`set_workflow`] has +/// nowhere to go and is dropped. +fn push_step(workflow: &mut Option, step: Step) { + if let Some(workflow) = workflow.as_mut() { + if workflow.steps.len() < MAX_STEPS { + workflow.steps.push(step); + } + } +} + +/// This run's event id, fixed on first use so a workflow can hand it to the +/// steps it launches before the event itself is built. +pub(crate) fn event_id() -> &'static str { + EVENT_ID.get_or_init(|| uuid_v4(rand::random())) +} + +/// The run that started this one, when a workflow step set it. Read back +/// only when it has the shape of an event id. +fn parent_event_id() -> Option { + let value = std::env::var(PARENT_EVENT_ENV).ok()?; + let value = value.trim(); + is_uuid(value).then(|| value.to_string()) +} + +fn sum(total: Option, more: Option) -> Option { + match (total, more) { + (Some(a), Some(b)) => Some(a.saturating_add(b)), + (a, b) => a.or(b), + } +} + +fn tilesets_word(word: &str) -> &str { + TILESETS_COMMANDS + .iter() + .find(|known| **known == word) + .copied() + .unwrap_or("other") +} + +fn clip_command(path: Vec) -> Vec { + path.into_iter() + .take(MAX_COMMAND_LEVELS) + .map(|name| clip(&name, MAX_COMMAND_NAME)) + .collect() +} + +fn classify( + name: &str, + values: &[String], + takes_values: bool, + enumerated: bool, + numeric: bool, +) -> Param { + let joined = values.join(","); + let mut param = Param { + name: clip(name, MAX_NAME), + ..Param::default() + }; + if COORDINATES.contains(&name) { + return param; + } + if !takes_values || enumerated || numeric || ALLOWLISTED.contains(&name) { + param.value = Some(clip(&joined, MAX_VALUE)); + return param; + } + match name { + "file" => { + param.bytes = values + .iter() + .filter_map(|path| std::fs::metadata(path).ok()) + .map(|meta| meta.len()) + .reduce(u64::saturating_add); + } + "data" => { + let (bytes, keys) = data_shape(&joined); + param.bytes = bytes; + param.keys = keys; + } + _ => param.length = Some(joined.chars().count() as u64), + } + param +} + +/// Size and top-level keys of a `--data` body: inline JSON, `@`, or +/// `@-` (stdin, which is not read twice, so nothing but the name). +fn data_shape(value: &str) -> (Option, Option>) { + if value == "@-" { + return (None, None); + } + let text = match value.strip_prefix('@') { + Some(path) => { + let Ok(meta) = std::fs::metadata(path) else { + return (None, None); + }; + // Parsed only when small enough to be a request body worth + // describing; the size alone is still recorded above that. + if meta.len() > MAX_DATA_TO_PARSE { + return (Some(meta.len()), None); + } + match std::fs::read_to_string(path) { + Ok(text) => text, + Err(_) => return (Some(meta.len()), None), + } + } + None => value.to_string(), + }; + let keys = serde_json::from_str::(&text) + .ok() + .and_then(|json| { + json.as_object().map(|object| { + object + .keys() + .take(MAX_KEYS) + .map(|key| clip(key, MAX_NAME)) + .collect() + }) + }); + (Some(text.len() as u64), keys) +} + +/// How this binary was installed, from where it lives. Only the category +/// leaves; the path does not. `MAPBOX_INSTALL_DIR` moves an install-script +/// binary anywhere, so those read as `other`. +fn install_method(exe: Option<&Path>) -> &'static str { + let Some(exe) = exe else { + return "other"; + }; + let path = exe + .to_string_lossy() + .replace('\\', "/") + .to_ascii_lowercase(); + if path.contains("/cellar/") || path.contains("/homebrew/") { + "homebrew" + } else if path.contains("/.cargo/bin/") { + "cargo" + } else if path.contains("/.local/bin/") || path.contains("/programs/mapbox/") { + "install-script" + } else { + "other" + } +} + +fn clip(text: &str, max_chars: usize) -> String { + text.chars().take(max_chars).collect() +} + +/// The installation's random id, created on first use. Two first runs in +/// parallel can each create one; `create_new` makes the second read the +/// first's instead of replacing it. When nothing can be stored, the run +/// still gets an id — just not one the next run will share. +fn user_id() -> String { + if let Some(existing) = read_user_id() { + return existing; + } + let fresh = uuid_v4(rand::random()); + if telemetry_sink::create_state(USER_ID_FILE, &fresh) { + return fresh; + } + read_user_id().unwrap_or(fresh) +} + +fn read_user_id() -> Option { + telemetry_sink::read_state(USER_ID_FILE).filter(|id| is_uuid(id)) +} + +fn is_uuid(text: &str) -> bool { + text.len() == 36 + && text.char_indices().all(|(i, c)| match i { + 8 | 13 | 18 | 23 => c == '-', + _ => c.is_ascii_hexdigit(), + }) +} + +/// The version the last run recorded, when it differs from this one — the +/// first run after an upgrade. Checked against the same shape the update +/// check trusts, since it is read back from disk. +fn previous_version() -> Option { + let last = telemetry_sink::read_state(LAST_VERSION_FILE); + if last.as_deref() == Some(CURRENT) { + return None; + } + telemetry_sink::replace_state(LAST_VERSION_FILE, CURRENT); + last.filter(|version| is_version(version)) + .map(|version| clip(&version, MAX_VERSION)) +} + +fn is_version(text: &str) -> bool { + !text.is_empty() + && text.len() <= MAX_VERSION + && text + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')) +} + +fn timestamp(at: SystemTime) -> String { + crate::dated_jsonl::timestamp(at) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::UNIX_EPOCH; + + #[test] + fn a_script_step_records_only_how_it_exited_and_how_long_it_took() { + let step = script_step(Some(3), Duration::from_millis(2300)); + assert_eq!( + serde_json::to_value(&step).unwrap(), + serde_json::json!({ "kind": "script", "exitCode": 3, "durationMs": 2300 }) + ); + } + + #[test] + fn a_cli_step_keeps_only_names_the_command_tree_has() { + let app = Command::new("mapbox") + .subcommand(Command::new("styles").subcommand(Command::new("get"))); + let step = cli_step( + &app, + &["styles", "get", "my-secret-style"], + Some(0), + None, + Duration::ZERO, + ); + assert_eq!( + step.command, + Some(vec!["styles".to_string(), "get".to_string()]) + ); + + let unknown = cli_step( + &app, + &["/Users/someone/run.sh"], + Some(1), + Some("error"), + Duration::ZERO, + ); + assert_eq!(unknown.command, None); + assert_eq!(unknown.error_code.as_deref(), Some("error")); + } + + #[test] + fn steps_need_a_workflow_and_stop_at_the_bound() { + let mut workflow = None; + push_step(&mut workflow, script_step(Some(0), Duration::ZERO)); + assert!(workflow.is_none(), "a step without a workflow is dropped"); + + workflow = Some(workflow_field(WorkflowSource::Builtin, Some("style-clone"))); + for _ in 0..MAX_STEPS + 5 { + push_step(&mut workflow, script_step(Some(0), Duration::ZERO)); + } + assert_eq!(workflow.unwrap().steps.len(), MAX_STEPS); + } + + #[test] + fn a_custom_workflow_never_records_its_name() { + assert_eq!( + workflow_field(WorkflowSource::Custom, Some("acme-client-export")).name, + None + ); + assert_eq!( + workflow_field(WorkflowSource::Marketplace, Some("style-clone")).name, + Some("style-clone".to_string()) + ); + assert_eq!(workflow_field(WorkflowSource::Builtin, None).name, None); + } + + #[test] + fn a_uuid_is_version_4_and_well_formed() { + let id = uuid_v4([0xff; 16]); + assert!(is_uuid(&id), "{id}"); + assert_eq!(&id[14..15], "4"); + assert!(matches!(&id[19..20], "8" | "9" | "a" | "b"), "{id}"); + assert!(!is_uuid("not-a-uuid")); + } + + #[test] + fn timestamps_are_utc_to_the_millisecond() { + let at = UNIX_EPOCH + Duration::from_millis(1_790_000_000_123); + assert_eq!(timestamp(at), "2026-09-21T14:13:20.123Z"); + assert_eq!(timestamp(UNIX_EPOCH), "1970-01-01T00:00:00.000Z"); + } + + #[test] + fn free_strings_send_their_length_and_codes_send_their_value() { + let values = |v: &str| vec![v.to_string()]; + let q = classify("q", &values("1600 Pennsylvania Ave"), true, false, false); + assert_eq!(q.value, None); + assert_eq!(q.length, Some(21)); + + // Digits are still a free string unless the spec types them. + let postcode = classify("postcode", &values("10001"), true, false, false); + assert_eq!((postcode.value, postcode.length), (None, Some(5))); + + let limit = classify("limit", &values("5"), true, false, true); + assert_eq!(limit.value.as_deref(), Some("5")); + + let language = classify("language", &values("en"), true, false, false); + assert_eq!(language.value.as_deref(), Some("en")); + + let flag = classify("download", &values("true"), false, false, false); + assert_eq!(flag.value.as_deref(), Some("true")); + } + + #[test] + fn coordinates_send_their_name_only() { + for name in COORDINATES { + let param = classify(name, &["12.5".to_string()], true, false, true); + assert_eq!( + param, + Param { + name: name.to_string(), + ..Param::default() + } + ); + } + } + + #[test] + fn a_data_body_sends_its_size_and_top_level_keys_only() { + let (bytes, keys) = data_shape(r#"{"name":"secret","layers":[]}"#); + assert_eq!(bytes, Some(29)); + assert_eq!(keys, Some(vec!["layers".to_string(), "name".to_string()])); + + assert_eq!(data_shape("[1,2]"), (Some(5), None)); + assert_eq!(data_shape("@-"), (None, None)); + } + + #[test] + fn a_long_value_is_clipped_to_the_schema_bound() { + let param = classify("types", &["x".repeat(500)], true, false, false); + assert_eq!(param.value.map(|v| v.len()), Some(MAX_VALUE)); + } + + #[test] + fn an_unknown_tilesets_word_is_other() { + assert_eq!(tilesets_word("upload-source"), "upload-source"); + assert_eq!(tilesets_word("my-secret-tileset"), "other"); + } + + #[test] + fn the_install_method_is_a_category_never_the_path() { + let method = |p: &str| install_method(Some(Path::new(p))); + assert_eq!( + method("/opt/homebrew/Cellar/mapbox/0.3.0/bin/mapbox"), + "homebrew" + ); + assert_eq!(method("/Users/a/.cargo/bin/mapbox"), "cargo"); + assert_eq!(method("/home/a/.local/bin/mapbox"), "install-script"); + assert_eq!( + method(r"C:\Users\a\AppData\Local\Programs\mapbox\mapbox.exe"), + "install-script" + ); + assert_eq!(method("/srv/tools/mapbox"), "other"); + assert_eq!(install_method(None), "other"); + } +} diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs new file mode 100644 index 0000000..3ea749b --- /dev/null +++ b/src/telemetry_sink.rs @@ -0,0 +1,138 @@ +//! Where the run's telemetry event goes once [`crate::telemetry_event`] has +//! built it. +//! +//! This module never decides what an event contains; it is handed one +//! finished JSON line. That keeps the privacy rules in one file and lets a +//! delivery change — a new endpoint, batching — happen without touching them. +//! +//! [`selected`] is the one place that picks the [`Sink`]. Today that is +//! [`FileSink`], which keeps events on this machine for local testing; +//! [`HttpSink`] is the interface for sending them, not implemented yet. +//! +//! It also owns the directory, `~/.mapbox/.telemetry` (or under +//! `$MAPBOX_CONFIG_DIR`), written only once the config directory exists, +//! including the two small state files the event reads — the installation +//! id and the last version seen. The dated event files are written and +//! pruned through [`crate::dated_jsonl`]. + +use std::io::Write; +use std::path::{Path, PathBuf}; + +use crate::auth; +use crate::dated_jsonl; + +const DIR: &str = ".telemetry"; +/// Days of event files [`FileSink`] keeps, today included. +const KEEP_DAYS: u64 = 7; + +/// Something that takes one finished event line and delivers it. +/// +/// Best-effort by contract: `deliver` reports nothing and must not fail the +/// command, block its exit, or write to stdout. +pub(crate) trait Sink { + fn deliver(&self, line: &str); +} + +/// Appends each event to `.jsonl`, one line per run. +pub(crate) struct FileSink; + +/// Sends each event to Mapbox Events. **Not implemented yet**: `deliver` +/// drops the event. +/// +/// What an implementation is expected to do: +/// +/// - `POST https://events.mapbox.com/events/v2?access_token=` with +/// the body `[]`, using a CLI-owned `pk.` token compiled into +/// release builds — never the user's. +/// - Send from a detached child so the command never waits, the way +/// `update_check` detaches its refresher, with the event on the child's +/// stdin rather than argv (argv is visible in `ps`). +/// - One attempt with a short timeout; drop the event on any failure. +/// - Send through [`crate::http::send`] with a `User-Agent` of +/// `mapbox-cli/` alone: Mapbox Events stores it in every record, +/// so the `agent/` marker must not ride along. +// Not constructed until `selected` switches to it. +#[allow(dead_code)] +pub(crate) struct HttpSink; + +impl Sink for FileSink { + fn deliver(&self, line: &str) { + if let Some(dir) = dir() { + dated_jsonl::append(&dir, line, KEEP_DAYS); + } + } +} + +impl Sink for HttpSink { + fn deliver(&self, _line: &str) {} +} + +/// Hands `line` to the [`selected`] sink. +pub(crate) fn deliver(line: &str) { + selected().deliver(line); +} + +/// The sink every event goes to. `HttpSink` replaces `FileSink` here once +/// it is implemented and `cli.command` is registered with Mapbox Events. +fn selected() -> &'static dyn Sink { + &FileSink +} + +/// The contents of a state file, trimmed, if it is there. +pub(crate) fn read_state(name: &str) -> Option { + let text = std::fs::read_to_string(dir()?.join(name)).ok()?; + Some(text.trim().to_string()) +} + +/// Creates a state file, only if it does not exist yet. `false` when it +/// already did or could not be written — two first runs racing each get +/// one answer this way, rather than the second replacing the first. +pub(crate) fn create_state(name: &str, contents: &str) -> bool { + let Some(dir) = dir() else { + return false; + }; + match create_private(&dir.join(name)) { + Ok(mut file) => file.write_all(contents.as_bytes()).is_ok(), + Err(_) => false, + } +} + +/// Replaces a state file's contents. +pub(crate) fn replace_state(name: &str, contents: &str) { + let Some(dir) = dir() else { + return; + }; + let path = dir.join(name); + let _ = std::fs::remove_file(&path); + if let Ok(mut file) = create_private(&path) { + let _ = file.write_all(contents.as_bytes()); + } +} + +/// The directory, created `0700` inside the config directory — only when +/// that already exists. Recording must not be what creates `~/.mapbox` or +/// changes its permissions: read-only commands promise to leave it alone +/// (`tests/auth_profiles.rs`, `tests/non_interactive.rs`). `None` otherwise, +/// and nothing is written. +/// +/// Stricter than command history, which creates a missing config directory +/// because history has to work for someone who has never logged in; this +/// file is a stand-in until the event is sent. +/// Creates `path` `0600`, failing if it already exists. +fn create_private(path: &Path) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + +fn dir() -> Option { + if !auth::config_dir_path()?.is_dir() { + return None; + } + dated_jsonl::private_dir(DIR) +} diff --git a/tests/history.rs b/tests/history.rs index 99aad4c..5e79db8 100644 --- a/tests/history.rs +++ b/tests/history.rs @@ -47,6 +47,8 @@ fn command(home: &Path) -> Command { .env_remove("NO_PROXY") .env_remove("no_proxy") .env("MAPBOX_NO_UPDATE_CHECK", "1") + // Telemetry keeps its own directory beside `history/`. + .env("MAPBOX_CLI_NO_TELEMETRY", "1") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) .env("MAPBOX_CONFIG_DIR", config_dir(home)); diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 3526c4c..ea8f220 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -47,6 +47,7 @@ fn sources() -> Vec<(String, String)> { /// size limit, matched by exact `YYYY-MM-DD.jsonl` names inside the /// directory it writes to, and the scratch file a trim leaves when its /// rename fails. +/// - `telemetry_sink` — a state file in `~/.mapbox/.telemetry` it replaces. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. @@ -58,6 +59,7 @@ const MAY_DELETE: &[&str] = &[ "executor.rs", "generate_skills.rs", "skill_dest.rs", + "telemetry_sink.rs", "uninstall.rs", ]; diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs new file mode 100644 index 0000000..2b28cf3 --- /dev/null +++ b/tests/telemetry_events.rs @@ -0,0 +1,272 @@ +//! End-to-end tests for the run's `cli.command` event. +//! +//! The unit tests in `src/telemetry_event.rs` cover the pure parts — how an argument +//! is classified, what a timestamp looks like, which files pruning may +//! touch. What they cannot show is what a real run leaves behind: that the +//! event lands where it should, carries what the command did and nothing the +//! user typed, disappears when telemetry is off, and never changes stdout. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +use serde_json::Value; + +/// A token whose payload claims the account `example-user`. The signature +/// is the part that must never appear in an event. +const TOKEN: &str = "pk.eyJ1IjoiZXhhbXBsZS11c2VyIiwiYSI6IngifQ.SIGNATURE-NOT-FOR-EVENTS"; +const ADDRESS: &str = "1600 Pennsylvania Ave"; + +fn scratch(name: &str) -> PathBuf { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("events-{name}")); + let _ = std::fs::remove_dir_all(&home); + // With `.mapbox` already there, as on any machine that has logged in: + // the file sink never creates it. + std::fs::create_dir_all(config_dir(&home)).expect("create the scratch config dir"); + home +} + +fn config_dir(home: &Path) -> PathBuf { + home.join(".mapbox") +} + +fn command(home: &Path) -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + cmd.env_remove("MAPBOX_ACCESS_TOKEN") + .env_remove("MapboxAccessToken") + .env_remove("MAPBOX_USERNAME") + .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_CLI_NO_TELEMETRY") + .env("MAPBOX_NO_UPDATE_CHECK", "1") + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", config_dir(home)); + cmd +} + +fn run(home: &Path, args: &[&str]) -> Output { + command(home).args(args).output().expect("run mapbox") +} + +/// Every event written under `home`, oldest first. +fn events(home: &Path) -> Vec { + let dir = config_dir(home).join(".telemetry"); + let Ok(entries) = std::fs::read_dir(&dir) else { + return vec![]; + }; + let mut files: Vec = entries + .map(|e| e.expect("an entry").path()) + .filter(|p| p.extension().is_some_and(|ext| ext == "jsonl")) + .collect(); + files.sort(); + files + .iter() + .flat_map(|f| { + std::fs::read_to_string(f) + .expect("read an event file") + .lines() + .map(|l| serde_json::from_str(l).expect("an event line is JSON")) + .collect::>() + }) + .collect() +} + +#[test] +fn a_run_writes_one_event_with_what_it_did() { + let home = scratch("one"); + let data = r#"{"name":"secret-style-name","layers":[]}"#; + let out = run( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + data, + "--dry-run", + "-t", + TOKEN, + ], + ); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + + let events = events(&home); + assert_eq!(events.len(), 1, "{events:?}"); + let event = &events[0]; + assert_eq!(event["event"], "cli.command"); + assert_eq!(event["sdkIdentifier"], "mapbox-cli"); + assert_eq!(event["command"], serde_json::json!(["styles", "create"])); + assert_eq!(event["invocation"], "execute"); + assert_eq!(event["exitCode"], 0); + assert_eq!(event["dryRun"], true); + assert_eq!(event["auth"]["source"], "flag"); + assert_eq!(event["auth"]["type"], "pk"); + assert_eq!(event["auth"]["account"], "example-user"); + assert_eq!( + event["stdoutBytes"].as_u64(), + Some(out.stdout.len() as u64), + "stdoutBytes should be what was written" + ); + let params = event["params"].as_array().expect("params"); + assert!(params.contains(&serde_json::json!({ + "name": "data", "bytes": data.len(), "keys": ["layers", "name"] + }))); + assert!(params.contains(&serde_json::json!({ "name": "username", "length": 7 }))); +} + +#[test] +fn nothing_the_user_typed_reaches_the_event() { + let home = scratch("private"); + // No network needed: a usage error still records, and `--dry-run` sends + // nothing. + let _ = run( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + "{\"x\":1}", + "--dry-run", + "-t", + TOKEN, + ], + ); + let _ = run( + &home, + &[ + "geocoder", + "forward", + "--q", + ADDRESS, + "--no-such-flag", + "-t", + TOKEN, + ], + ); + let _ = run(&home, &["/Users/someone/secret/path"]); + + let written = std::fs::read_dir(config_dir(&home).join(".telemetry")) + .expect("the telemetry directory") + .map(|e| std::fs::read_to_string(e.expect("an entry").path()).unwrap_or_default()) + .collect::(); + for secret in [ + "SIGNATURE-NOT-FOR-EVENTS", + ADDRESS, + "someone", + "secret/path", + ] { + assert!( + !written.contains(secret), + "`{secret}` reached the event files:\n{written}" + ); + } + assert_eq!(events(&home).len(), 3); +} + +#[test] +fn help_version_and_usage_errors_record_their_invocation() { + let home = scratch("invocation"); + let _ = run(&home, &["--version"]); + let _ = run(&home, &["styles", "--help"]); + let _ = run(&home, &["styles", "list", "--schema"]); + let _ = run(&home, &["nosuchcommand"]); + + let events = events(&home); + let seen: Vec<(&str, &Value)> = events + .iter() + .map(|e| (e["invocation"].as_str().unwrap_or(""), &e["command"])) + .collect(); + assert_eq!( + seen, + [ + ("version", &Value::Null), + ("help", &serde_json::json!(["styles"])), + ("schema", &serde_json::json!(["styles", "list"])), + ("execute", &Value::Null), + ] + ); + assert_eq!(events[3]["usageError"], "InvalidSubcommand"); + assert_eq!(events[3]["errorCode"], "usage"); + assert_eq!(events[3]["exitCode"], 2); +} + +#[test] +fn the_opt_out_records_nothing() { + let home = scratch("opt-out-env"); + let out = command(&home) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "MAPBOX_CLI_NO_TELEMETRY=1 still wrote telemetry" + ); +} + +#[test] +fn stdout_is_identical_with_telemetry_on_and_off() { + let on = run(&scratch("stdout-on"), &["-o", "json", "config", "list"]); + let off = command(&scratch("stdout-off")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["-o", "json", "config", "list"]) + .output() + .expect("run mapbox"); + assert_eq!(on.stdout, off.stdout); + assert_eq!(on.stderr, off.stderr); +} + +#[test] +fn completion_records_nothing() { + let home = scratch("completion"); + assert!(run(&home, &["completion", "zsh"]).status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "`completion` wrote telemetry" + ); +} + +/// Recording never creates the config directory: a machine that has never +/// logged in or set a config keeps no `~/.mapbox` at all. +#[test] +fn without_a_config_directory_nothing_is_created() { + let home = scratch("no-config-dir"); + std::fs::remove_dir(config_dir(&home)).expect("remove the scratch config dir"); + assert!(run(&home, &["styles", "--help"]).status.success()); + assert!( + !config_dir(&home).exists(), + "recording created {}", + config_dir(&home).display() + ); +} + +#[test] +fn a_run_started_by_a_workflow_step_records_its_parent() { + let parent = "5f0c1e9a-7b2d-4c1e-9f3a-2d8e6b1a0c47"; + let home = scratch("parent"); + let out = command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", parent) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + // Anything that isn't an event id is ignored rather than recorded. + let _ = command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", "/Users/someone/secret") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + + let events = events(&home); + assert_eq!(events.len(), 2); + assert_eq!(events[0]["parentEventId"], parent); + assert_ne!(events[0]["eventId"], parent); + assert!(events[1].get("parentEventId").is_none(), "{:?}", events[1]); +}