WIP: Add a persisted telemetry setting to mapbox config

CHANGELOG.md

@@ -19,6 +19,10 @@ that may never merge. They are not releases and are not listed here.
 
 ## Unreleased
 
+- `mapbox config set telemetry off` turns the telemetry event off for good,
+  in every shell, the way `MAPBOX_CLI_NO_TELEMETRY=1` does for one. The run
+  that turns it off records nothing either. `mapbox config list` now reports
+  `telemetry` alongside `update-check`.
 - Each run records one `cli.command` telemetry event, appended to
   `~/.mapbox/.telemetry/<date>.jsonl` and kept for 7 days. Nothing is sent
   anywhere by default. It never touches stdout, the exit code or how long a

README.md

@@ -208,7 +208,8 @@ Each run also appends one event to `~/.mapbox/.telemetry/<date>.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.
+It stays on this machine. `MAPBOX_CLI_NO_TELEMETRY=1` or
+`mapbox config set telemetry off` turns it off.
 
 ### Agent skills
 

docs/commands.md

@@ -3194,10 +3194,12 @@ 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
+(or `$MAPBOX_CONFIG_DIR`), written the same way credentials are. Two
+settings: `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.
+rather than only the one the environment variable was set in, and
+`telemetry`, which does the same for the run's telemetry event and
+`MAPBOX_CLI_NO_TELEMETRY`.
 
 ### `mapbox config get`
 
@@ -3209,7 +3211,7 @@ than failing, the same forgiving read the update-check cache itself uses.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to read. Only `update-check` exists today. |
+| `<key>` | Which setting to read: `update-check` or `telemetry`. |
 
 #### Examples
 
@@ -3248,7 +3250,7 @@ without an environment variable.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to change. Only `update-check` exists today. |
+| `<key>` | Which setting to change: `update-check` or `telemetry`. |
 | `<value>` | `on` or `off`. |
 
 #### Examples
@@ -3332,7 +3334,7 @@ default, a key explicitly set to the old default value does not.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to clear. Only `update-check` exists today. |
+| `<key>` | Which setting to clear: `update-check` or `telemetry`. |
 
 #### Examples
 

src/config.rs

@@ -6,8 +6,8 @@
 //! 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
+//! Two settings — `update-check` and `telemetry` — with room for more:
+//! `get`/`set`/`unset` take a `key`, restricted by clap to [`KEYS`], so a new
 //! 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
@@ -30,7 +30,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 TELEMETRY_KEY: &str = "telemetry";
+const KEYS: &[&str] = &[UPDATE_CHECK_KEY, TELEMETRY_KEY];
 
 const ON: &str = "on";
 const OFF: &str = "off";
@@ -43,6 +44,8 @@ const OFF: &str = "off";
 struct Config {
     #[serde(default, skip_serializing_if = "Option::is_none")]
     update_check: Option<bool>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    telemetry: Option<bool>,
 }
 
 fn config_path() -> Option<PathBuf> {
@@ -83,6 +86,12 @@ pub fn update_check_enabled() -> bool {
     update_check_setting(&read_config())
 }
 
+/// Whether the run's telemetry event may be recorded, per the persisted
+/// setting. [`crate::telemetry_event`] checks it alongside `MAPBOX_CLI_NO_TELEMETRY`.
+pub fn telemetry_enabled() -> bool {
+    read_config().telemetry.unwrap_or(true)
+}
+
 fn on_off(enabled: bool) -> &'static str {
     if enabled {
         ON
@@ -97,6 +106,7 @@ fn on_off(enabled: bool) -> &'static str {
 fn resolve(config: &Config, key: &str) -> bool {
     match key {
         UPDATE_CHECK_KEY => update_check_setting(config),
+        TELEMETRY_KEY => config.telemetry.unwrap_or(true),
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
 }
@@ -109,6 +119,7 @@ fn resolve(config: &Config, key: &str) -> bool {
 fn clear(config: &mut Config, key: &str) {
     match key {
         UPDATE_CHECK_KEY => config.update_check = None,
+        TELEMETRY_KEY => config.telemetry = None,
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
 }
@@ -173,6 +184,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),
+        TELEMETRY_KEY => config.telemetry = Some(enabled),
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
     write_config(&config)?;
@@ -228,6 +240,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 +262,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,13 +281,15 @@ 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());
         assert_eq!(explicit_default.update_check, None);
 
         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);

src/telemetry_event.rs

@@ -16,7 +16,8 @@
 //!
 //! 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 recorded or written.
+//! (`MAPBOX_CLI_NO_TELEMETRY`, or `mapbox config set telemetry off`),
+//! nothing is recorded or written.
 
 use std::collections::HashSet;
 use std::io::IsTerminal;
@@ -354,7 +355,7 @@ static ENABLED: OnceLock<bool> = OnceLock::new();
 
 /// Read once: the answer must not change halfway through a run.
 fn enabled() -> bool {
-    *ENABLED.get_or_init(telemetry::telemetry_allowed)
+    *ENABLED.get_or_init(|| telemetry::telemetry_allowed() && crate::config::telemetry_enabled())
 }
 
 /// A poisoned lock is a panic somewhere else; recording is not worth a
@@ -624,7 +625,9 @@ pub fn finish(exit_code: Option<u32>) {
 }
 
 fn deliver_locked(run: &mut Run, exit_code: Option<u32>) {
-    if run.finished || !enabled() {
+    // The setting is read again here: the run that turns telemetry off is
+    // one that should not report itself.
+    if run.finished || !enabled() || !crate::config::telemetry_enabled() {
         return;
     }
     run.finished = true;

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":"telemetry","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":"telemetry","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\ntelemetry\ton");
 }
 
 #[test]

tests/telemetry_events.rs

@@ -197,7 +197,7 @@ fn help_version_and_usage_errors_record_their_invocation() {
 }
 
 #[test]
-fn the_opt_out_records_nothing() {
+fn either_opt_out_records_nothing() {
     let home = scratch("opt-out-env");
     let out = command(&home)
         .env("MAPBOX_CLI_NO_TELEMETRY", "1")
@@ -209,6 +209,14 @@ fn the_opt_out_records_nothing() {
         !config_dir(&home).join(".telemetry").exists(),
         "MAPBOX_CLI_NO_TELEMETRY=1 still wrote telemetry"
     );
+
+    let home = scratch("opt-out-config");
+    assert!(run(&home, &["config", "set", "telemetry", "off"])
+        .status
+        .success());
+    // That run records nothing either: the setting it wrote is read at exit.
+    let _ = run(&home, &["config", "list"]);
+    assert_eq!(events(&home), Vec::<Value>::new());
 }
 
 #[test]