feat(cli): add prompt and auto update modes, and defer to the installing package manager

Adds the two remaining rungs of the update ladder. `prompt` asks once, after a
command that succeeded, on a real terminal only. `auto` installs at the end of
the run, but only for a binary Perry itself installed: a Homebrew, npm, apt or
winget copy prints that tool's own upgrade command instead of overwriting a file
the package manager owns. `perry update --mode <mode>` writes the setting, and
`perry doctor` reports it.

Author: jdalton

changelog.d/7749-update-config-surface.md

@@ -27,10 +27,11 @@ prompt_default = false     # what Enter means in prompt mode
 ```
 
 `off` and `notify` are the two behaviours that already existed, and `notify` is
-the default, so a user who never opens the config sees no change. `prompt` and
-`auto` are accepted and documented here but not yet wired to an install — that
-is the next slice, deliberately separate, because replacing the binary a user
-is running deserves its own review.
+the default, so a user who never opens the config sees no change. `prompt` asks once,
+after a command that succeeded, on a real terminal only; `auto` installs at the
+end of the run, and only for a binary Perry itself installed. Those two arrived
+in a separate change from this one, because replacing the binary a user is
+running deserves its own review.
 
 `PERRY_UPDATE_MODE` sets the same thing for one run.
 

changelog.d/7784-update-prompt-auto-and-channels.md

@@ -0,0 +1,105 @@
+### Added
+
+**`prompt` and `auto` update modes now do something, and refuse to do the wrong
+thing.** The previous slice made the modes configurable; this wires them to the
+existing signed self-updater, behind three refusals.
+
+**A package-managed install is never replaced in place.** `perry update`
+overwrites the running executable, which is right for a tarball or `install.sh`
+install and wrong for every managed one: Homebrew, npm, apt and winget each keep
+their own record of what is installed and at what version, and overwriting the
+file underneath leaves that record lying. `prompt` and `auto` now detect the
+owner and name that owner's command instead:
+
+| owner | what Perry says to run |
+|---|---|
+| Homebrew | `brew upgrade perryts/perry/perry` |
+| npm | `npm install -g @perryts/perry@latest` |
+| apt | `sudo apt update && sudo apt install --only-upgrade perry` |
+| winget | `winget upgrade PerryTS.Perry` |
+
+npm gets an extra sentence, because it is the worst case: Perry ships as a
+wrapper package plus a per-platform binary package, so replacing the binary also
+desyncs it from the wrapper that launched it.
+
+**Nothing is offered after a command that failed.** The user is looking at an
+error; a question about upgrading is noise at the worst possible moment, and an
+unattended install would bury the error under progress output. Both active modes
+fall back to a plain notice.
+
+**An unwritable install directory is reported, not attempted.** `install.sh`
+targets `/usr/local/bin`, which is root-owned on a default macOS and most Linux
+boxes. That is now checked *before* anything is downloaded, so the outcome is
+one sentence naming `sudo perry update` rather than a half-finished install. Perry
+never escalates on its own.
+
+**`perry update --mode <off|notify|prompt|auto>`** saves the setting and exits,
+so the one thing people are most likely to change does not require hand-editing
+TOML. It is a read-modify-write through the shared loader, so the rest of the
+file comes back out the way it went in.
+
+**`perry doctor`** now reports the effective mode and, when there is one, the
+package manager that owns the binary — the two questions behind "why did it not
+update".
+
+<details>
+<summary><b>Why the channel detection fails open</b></summary>
+
+Every rule answers "is this definitely managed?", never "is this definitely
+unmanaged?", and an unrecognised layout resolves to self-managed.
+
+That asymmetry is deliberate. Guessing "managed" wrongly would refuse to
+self-update a plain tarball install — the majority case, and the one with no
+other upgrade path. Guessing "self-managed" wrongly costs an in-place update on
+a machine that had a package manager available, which is recoverable by running
+that manager.
+
+The paths are canonicalized before classification, because Homebrew's `perry` in
+`/usr/local/bin` is a symlink into the Cellar; classifying the link rather than
+its target would miss every Homebrew install there is.
+
+apt requires **both** a dpkg file list and a dpkg-owned path, because dpkg does
+not own `/usr/local` — that is `install.sh`'s directory. The path alone would
+misclassify a hand-placed binary; the dpkg list alone would claim a tarball
+install on a machine that also has the `.deb` installed somewhere else. The check
+is a file-existence test rather than a `dpkg -S` subprocess, since this runs on
+the update path of every command.
+</details>
+
+<details>
+<summary><b>Prompting needs stdin, not just stderr</b></summary>
+
+The mode gate already requires stderr to be a terminal. That is not enough to
+ask a question: stdin can be a pipe while stderr is a tty, and reading from it
+would either block the command or take whatever the pipe happened to contain as
+consent. `prompt` degrades to a plain notice when stdin is not a terminal.
+
+`auto` asks nothing, so it does not need stdin — but it does still require the
+command to have succeeded, an unmanaged install, and a writable directory.
+</details>
+
+<details>
+<summary><b>Tests</b></summary>
+
+24 new, all in the required per-pull-request job. The decision is a pure
+function of the mode plus four facts about the machine, so every refusal is
+asserted directly rather than left inside an `if` in the middle of a teardown
+path:
+
+- both active modes downgrade to a notice after a failed command;
+- both refuse on all four managed channels, and name a command for each;
+- both report elevation rather than attempting an unwritable install;
+- `prompt` degrades without stdin while `auto` does not need it.
+
+The channel table covers Homebrew under all three prefixes, npm for global, nvm
+and project-local layouts, apt with and without each half of its rule, both
+winget delivery shapes, and four unrecognised layouts that must fail open.
+Classification splits on both path separators rather than using
+`Path::components`, so the winget cases run on every host instead of only on
+Windows.
+
+Verified end to end: writing `mode` into a real config file that already had a
+`license_key` and an unknown `[update] future_key` left both intact.
+
+`cargo test -p perry`: 914 passed, 0 failed.
+</details>

changelog.d/7787-update-surface-followups.md

@@ -0,0 +1,55 @@
+### Fixed
+
+Five defects in the update surface, all found in review of
+[#7749](https://github.com/PerryTS/perry/pull/7749) after it had merged.
+
+**The config warning escaped the rules that were meant to silence it.** The
+"unrecognized `[update] mode`" line was printed inside `UpdatePolicy::resolve`,
+before the precedence rules it sits behind had been applied — so it reached
+stderr during `--format json`, in CI, with a piped stderr, and under `--quiet`.
+Those rules exist to keep exactly those runs silent, and the one line whose job
+was to report a config problem was the one line ignoring them. It is now held on
+the policy and emitted at the single point where the run is known to be speaking
+at all.
+
+**The notify interval throttled on time alone, so it swallowed the next
+release.** The documented contract is that the interval throttles repeats of
+*the same* update. Keyed only on a timestamp, it also suppressed a **different**
+version that arrived inside the window — so somebody setting a week-long
+interval to stop being nagged about one release would also have been denied the
+release that fixed it. The cache now records which version it announced, and a
+different version is announced regardless of the interval.
+
+**The interval comparison was signed.** `Duration::as_secs() as i64` goes
+negative for a large enough configured value, and a negative interval reads as
+already-elapsed — so an absurd value would have notified on *every* run instead
+of suppressing. The comparison is unsigned.
+
+**Two `perry` processes could corrupt the cache.** Every write used one shared
+`*.json.tmp`, so two writers each wrote it and each renamed it: the loser's
+rename landed a file the winner was still writing into. Each write now builds
+its own temporary name.
+
+**A refresh could erase a notice recorded while its request was in flight.**
+`fetch_latest_version` read the notice state *before* issuing its request and
+wrote it back afterwards, overwriting anything recorded in between — telling the
+user about the same release twice. The read-modify-write pairs are now
+serialized by a lock file, and the refresh re-reads inside that lock immediately
+before replacing.
+
+<details>
+<summary><b>Tests</b></summary>
+
+Two new contract tests, both sabotage-verified — reverting either fix turns its
+test red:
+
+- a different version is announced regardless of the interval, and never having
+  announced anything counts as "not this version";
+- an enormous interval still suppresses rather than wrapping into notifying.
+
+The existing interval tests are unchanged in intent: they now go through a
+helper that holds the announced version constant, so they still exercise only
+the interval arithmetic.
+
+`cargo test -p perry`: 904 passed, 0 failed.
+</details>

crates/perry/src/commands/doctor.rs

@@ -286,16 +286,41 @@ fn check_runtime_library() -> CheckResult {
 }
 
 fn check_update_available() -> CheckResult {
+    // Say which mode is in effect and who owns this binary. Both are things a
+    // user asks `doctor` about precisely when updates are not behaving as they
+    // expect — "why did it not install" is almost always one of the two.
+    let policy = crate::update_policy::UpdatePolicy::resolve();
+    let channel = crate::install_channel::detect();
+    // Configured, not effective: `doctor --format json` and `doctor | less` both
+    // suppress the update surface for their own run, so reporting the effective
+    // mode would answer "off" to the very question being asked.
+    let mut detail = format!(" (mode: {}", policy.configured_mode.label());
+    if !policy.is_active() && policy.configured_mode != crate::update_policy::UpdateMode::Off {
+        detail.push_str("; suppressed for this run");
+    }
+    if let Some(command) = channel.upgrade_command() {
+        detail.push_str(&format!(
+            "; installed by {} — upgrade with `{}`",
+            channel.label(),
+            command
+        ));
+    }
+    detail.push(')');
+    let context = detail;
+
     match update_checker::check_cached_status() {
         update_checker::UpdateStatus::UpdateAvailable { latest, .. } => CheckResult {
             name: "update status".to_string(),
             status: CheckStatus::Warning,
-            details: Some(format!("v{} available — run `perry update`", latest)),
+            details: Some(format!(
+                "v{} available — run `perry update`{}",
+                latest, context
+            )),
         },
         update_checker::UpdateStatus::UpToDate => CheckResult {
             name: "update status".to_string(),
             status: CheckStatus::Ok,
-            details: Some("up to date".to_string()),
+            details: Some(format!("up to date{}", context)),
         },
         update_checker::UpdateStatus::CheckFailed => CheckResult {
             name: "update status".to_string(),

crates/perry/src/commands/publish/mod.rs

@@ -35,7 +35,8 @@ pub use args::PublishArgs;
 #[cfg(test)]
 pub(crate) use saved_config::IosSavedConfig; // consumed only by tests
 pub(crate) use saved_config::{
-    check_beta_consent, config_path, is_interactive, load_config, prompt_input, report_beta_error,
+    check_beta_consent, config_path, is_interactive, load_config, load_config_checked,
+    prompt_input, report_beta_error, update_config_file,
     save_config, AndroidSavedConfig, AppleSavedConfig, HarmonyosSavedConfig, PerryConfig,
 };
 pub(crate) use tarball::create_project_tarball_with_filters;

crates/perry/src/commands/publish/saved_config.rs

@@ -119,12 +119,39 @@ pub(crate) fn config_path() -> PathBuf {
 }
 
 pub(crate) fn load_config() -> PerryConfig {
+    load_config_checked().unwrap_or_default()
+}
+
+/// `load_config`, but able to say WHY it produced nothing.
+///
+/// `Err` means the file exists and does not parse — the case a plain
+/// `unwrap_or_default()` cannot tell apart from "no file yet". The difference
+/// matters at save time, because writing a default struct over a damaged but
+/// hand-recoverable config destroys the user's license key and tokens along with
+/// the syntax error they were about to fix.
+pub(crate) fn load_config_checked() -> std::result::Result<PerryConfig, String> {
     let path = config_path();
-    if let Ok(content) = fs::read_to_string(&path) {
-        toml::from_str(&content).unwrap_or_default()
-    } else {
-        PerryConfig::default()
-    }
+    let Ok(content) = fs::read_to_string(&path) else {
+        // No file, or unreadable. Defaults are right here.
+        return Ok(PerryConfig::default());
+    };
+    toml::from_str(&content).map_err(|error| error.to_string())
+}
+
+/// Read, modify, write — refusing to write when the read failed.
+///
+/// Every setting-writer goes through this. Handing `load_config`'s default
+/// struct to `save_config` is how a damaged file becomes an erased one, and a
+/// caller cannot tell the two apart on its own.
+pub(crate) fn update_config_file(edit: impl FnOnce(&mut PerryConfig)) -> Result<()> {
+    let mut config = load_config_checked().map_err(|error| {
+        anyhow::anyhow!(
+            "~/.perry/config.toml could not be parsed, so it was left untouched: \
+             {error}. Fix the file (or delete it) and try again."
+        )
+    })?;
+    edit(&mut config);
+    save_config(&config)
 }
 
 pub(crate) fn save_config(config: &PerryConfig) -> Result<()> {
@@ -326,3 +353,60 @@ check_interval_hours = 6
         assert!(written.contains("keep-me") && written.contains("[telemetry]"));
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A config that does not parse must be left alone, not replaced by defaults.
+    ///
+    /// This is the data-loss case. `load_config` cannot tell "no file yet" from
+    /// "damaged file", so a writer built on it turns one stray character into an
+    /// erased license key — the user's own tokens, gone while they were fixing a
+    /// typo.
+    #[test]
+    fn a_damaged_config_is_never_overwritten_with_defaults() {
+        let _lock = crate::test_env_lock::env_lock();
+        let home = tempfile::tempdir().expect("tempdir");
+        let saved = std::env::var_os("HOME");
+        std::env::set_var("HOME", home.path());
+
+        let path = config_path();
+        std::fs::create_dir_all(path.parent().expect("parent")).expect("mkdir");
+        let damaged = "license_key = \"keep-me\"\nthis is not toml\n";
+        std::fs::write(&path, damaged).expect("write");
+
+        let error = update_config_file(|config| {
+            config.update.get_or_insert_with(Default::default).mode =
+                Some(crate::update_policy::UpdateMode::Notify);
+        })
+        .expect_err("a damaged config must refuse the write");
+        assert!(
+            format!("{error:#}").contains("left untouched"),
+            "the message must say the file was not written: {error:#}"
+        );
+        assert_eq!(
+            std::fs::read_to_string(&path).expect("read"),
+            damaged,
+            "the file was rewritten despite the refusal"
+        );
+
+        // A missing file is still the "use defaults" case, so a first-time write
+        // has to succeed.
+        std::fs::remove_file(&path).expect("remove");
+        update_config_file(|config| {
+            config.update.get_or_insert_with(Default::default).mode =
+                Some(crate::update_policy::UpdateMode::Notify);
+        })
+        .expect("a fresh config must be writable");
+        assert!(
+            std::fs::read_to_string(&path).expect("read").contains("mode"),
+            "the setting was not persisted"
+        );
+
+        match saved {
+            Some(v) => std::env::set_var("HOME", v),
+            None => std::env::remove_var("HOME"),
+        }
+    }
+}

crates/perry/src/commands/update.rs

@@ -15,6 +15,14 @@ pub struct UpdateArgs {
     /// Ignore cache, always fetch from server
     #[arg(long)]
     pub force: bool,
+
+    /// Save how Perry should handle updates from now on, then exit.
+    ///
+    /// This is the writable half of `[update] mode` in ~/.perry/config.toml —
+    /// there to save people hand-editing TOML for the one setting they are
+    /// most likely to want to change.
+    #[arg(long, value_name = "off|notify|prompt|auto")]
+    pub mode: Option<String>,
 }
 
 pub fn run(
@@ -24,6 +32,10 @@ pub fn run(
     verbose: u8,
     quiet: bool,
 ) -> Result<()> {
+    if let Some(raw) = args.mode.as_deref() {
+        return set_mode(raw);
+    }
+
     let current = env!("CARGO_PKG_VERSION");
 
     let status = if !args.force && !update_checker::is_cache_stale() {
@@ -119,3 +131,34 @@ pub fn run(
 
     Ok(())
 }
+
+/// Persist `[update] mode`, and nothing else.
+///
+/// Read-modify-write through the shared loader so the rest of the file — the
+/// license key, the telemetry section, anything a newer Perry wrote — comes
+/// back out the way it went in.
+fn set_mode(raw: &str) -> Result<()> {
+    let Some(mode) = crate::update_policy::UpdateMode::parse(raw) else {
+        anyhow::bail!("unknown update mode `{raw}`. Valid values: off, notify, prompt, auto.");
+    };
+    if mode == crate::update_policy::UpdateMode::Unknown {
+        anyhow::bail!("unknown update mode `{raw}`. Valid values: off, notify, prompt, auto.");
+    }
+
+    crate::commands::publish::update_config_file(|config| {
+        config.update.get_or_insert_with(Default::default).mode = Some(mode);
+    })?;
+
+    let path = crate::commands::publish::config_path();
+    println!("Update mode set to \"{raw}\" ({}).", path.display());
+    if mode == crate::update_policy::UpdateMode::Auto {
+        // Say the limits up front rather than letting someone discover them
+        // the first time an update does not happen.
+        println!(
+            "Perry will install updates at the end of a successful run — except \
+             on a package-manager-managed install, where it names that manager's \
+             command instead."
+        );
+    }
+    Ok(())
+}