Skip to content

Commit b7e6988

Browse files
committed
feat(cli): wire prompt and auto update modes, behind three refusals
prompt and auto now reach the existing signed self-updater, but never on a package-managed install: Homebrew, npm, apt and winget each track what they installed, and overwriting the binary underneath leaves that record lying. The owner is detected and its own upgrade command named instead. npm says more, because Perry ships as a wrapper plus a per-platform binary package, so a replacement also desyncs the wrapper that launched it. Neither mode offers anything after a command that failed -- the user is reading an error, and an unattended install would bury it -- and an unwritable install directory is reported before anything is downloaded rather than failing halfway through a rename. Perry never escalates on its own. Channel detection fails open: every rule answers 'definitely managed?', so an unrecognized layout is treated as ours to replace. Guessing wrong the other way would refuse to update a tarball install, which is the majority case and the one with no alternative path. Paths are canonicalized first because Homebrew's bin entry is a symlink into the Cellar, and apt requires both a dpkg file list and a dpkg-owned path because dpkg does not own /usr/local. perry update --mode saves the setting through the shared loader, and doctor reports the effective mode plus the owning package manager.
1 parent a88bcdd commit b7e6988

6 files changed

Lines changed: 769 additions & 7 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
### Added
2+
3+
**`prompt` and `auto` update modes now do something, and refuse to do the wrong
4+
thing.** The previous slice made the modes configurable; this wires them to the
5+
existing signed self-updater, behind three refusals.
6+
7+
**A package-managed install is never replaced in place.** `perry update`
8+
overwrites the running executable, which is right for a tarball or `install.sh`
9+
install and wrong for every managed one: Homebrew, npm, apt and winget each keep
10+
their own record of what is installed and at what version, and overwriting the
11+
file underneath leaves that record lying. `prompt` and `auto` now detect the
12+
owner and name that owner's command instead:
13+
14+
| owner | what Perry says to run |
15+
|---|---|
16+
| Homebrew | `brew upgrade perryts/perry/perry` |
17+
| npm | `npm install -g @perryts/perry@latest` |
18+
| apt | `sudo apt update && sudo apt install --only-upgrade perry` |
19+
| winget | `winget upgrade PerryTS.Perry` |
20+
21+
npm gets an extra sentence, because it is the worst case: Perry ships as a
22+
wrapper package plus a per-platform binary package, so replacing the binary also
23+
desyncs it from the wrapper that launched it.
24+
25+
**Nothing is offered after a command that failed.** The user is looking at an
26+
error; a question about upgrading is noise at the worst possible moment, and an
27+
unattended install would bury the error under progress output. Both active modes
28+
fall back to a plain notice.
29+
30+
**An unwritable install directory is reported, not attempted.** `install.sh`
31+
targets `/usr/local/bin`, which is root-owned on a default macOS and most Linux
32+
boxes. That is now checked *before* anything is downloaded, so the outcome is
33+
one sentence naming `sudo perry update` rather than a half-finished install. Perry
34+
never escalates on its own.
35+
36+
**`perry update --mode <off|notify|prompt|auto>`** saves the setting and exits,
37+
so the one thing people are most likely to change does not require hand-editing
38+
TOML. It is a read-modify-write through the shared loader, so the rest of the
39+
file comes back out the way it went in.
40+
41+
**`perry doctor`** now reports the effective mode and, when there is one, the
42+
package manager that owns the binary — the two questions behind "why did it not
43+
update".
44+
45+
<details>
46+
<summary><b>Why the channel detection fails open</b></summary>
47+
48+
Every rule answers "is this definitely managed?", never "is this definitely
49+
unmanaged?", and an unrecognised layout resolves to self-managed.
50+
51+
That asymmetry is deliberate. Guessing "managed" wrongly would refuse to
52+
self-update a plain tarball install — the majority case, and the one with no
53+
other upgrade path. Guessing "self-managed" wrongly costs an in-place update on
54+
a machine that had a package manager available, which is recoverable by running
55+
that manager.
56+
57+
The paths are canonicalized before classification, because Homebrew's `perry` in
58+
`/usr/local/bin` is a symlink into the Cellar; classifying the link rather than
59+
its target would miss every Homebrew install there is.
60+
61+
apt requires **both** a dpkg file list and a dpkg-owned path, because dpkg does
62+
not own `/usr/local` — that is `install.sh`'s directory. The path alone would
63+
misclassify a hand-placed binary; the dpkg list alone would claim a tarball
64+
install on a machine that also has the `.deb` installed somewhere else. The check
65+
is a file-existence test rather than a `dpkg -S` subprocess, since this runs on
66+
the update path of every command.
67+
</details>
68+
69+
<details>
70+
<summary><b>Prompting needs stdin, not just stderr</b></summary>
71+
72+
The mode gate already requires stderr to be a terminal. That is not enough to
73+
ask a question: stdin can be a pipe while stderr is a tty, and reading from it
74+
would either block the command or take whatever the pipe happened to contain as
75+
consent. `prompt` degrades to a plain notice when stdin is not a terminal.
76+
77+
`auto` asks nothing, so it does not need stdin — but it does still require the
78+
command to have succeeded, an unmanaged install, and a writable directory.
79+
</details>
80+
81+
<details>
82+
<summary><b>Tests</b></summary>
83+
84+
24 new, all in the required per-pull-request job. The decision is a pure
85+
function of the mode plus four facts about the machine, so every refusal is
86+
asserted directly rather than left inside an `if` in the middle of a teardown
87+
path:
88+
89+
- both active modes downgrade to a notice after a failed command;
90+
- both refuse on all four managed channels, and name a command for each;
91+
- both report elevation rather than attempting an unwritable install;
92+
- `prompt` degrades without stdin while `auto` does not need it.
93+
94+
The channel table covers Homebrew under all three prefixes, npm for global, nvm
95+
and project-local layouts, apt with and without each half of its rule, both
96+
winget delivery shapes, and four unrecognised layouts that must fail open.
97+
Classification splits on both path separators rather than using
98+
`Path::components`, so the winget cases run on every host instead of only on
99+
Windows.
100+
101+
Verified end to end: writing `mode` into a real config file that already had a
102+
`license_key` and an unknown `[update] future_key` left both intact.
103+
104+
`cargo test -p perry`: 914 passed, 0 failed.
105+
</details>

crates/perry/src/commands/doctor.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,16 +286,34 @@ fn check_runtime_library() -> CheckResult {
286286
}
287287

288288
fn check_update_available() -> CheckResult {
289+
// Say which mode is in effect and who owns this binary. Both are things a
290+
// user asks `doctor` about precisely when updates are not behaving as they
291+
// expect — "why did it not install" is almost always one of the two.
292+
let policy = crate::update_policy::UpdatePolicy::resolve();
293+
let channel = crate::install_channel::detect();
294+
let context = match channel.upgrade_command() {
295+
Some(command) => format!(
296+
" (mode: {}; installed by {} — upgrade with `{}`)",
297+
policy.mode.label(),
298+
channel.label(),
299+
command
300+
),
301+
None => format!(" (mode: {})", policy.mode.label()),
302+
};
303+
289304
match update_checker::check_cached_status() {
290305
update_checker::UpdateStatus::UpdateAvailable { latest, .. } => CheckResult {
291306
name: "update status".to_string(),
292307
status: CheckStatus::Warning,
293-
details: Some(format!("v{} available — run `perry update`", latest)),
308+
details: Some(format!(
309+
"v{} available — run `perry update`{}",
310+
latest, context
311+
)),
294312
},
295313
update_checker::UpdateStatus::UpToDate => CheckResult {
296314
name: "update status".to_string(),
297315
status: CheckStatus::Ok,
298-
details: Some("up to date".to_string()),
316+
details: Some(format!("up to date{}", context)),
299317
},
300318
update_checker::UpdateStatus::CheckFailed => CheckResult {
301319
name: "update status".to_string(),

crates/perry/src/commands/update.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ pub struct UpdateArgs {
1515
/// Ignore cache, always fetch from server
1616
#[arg(long)]
1717
pub force: bool,
18+
19+
/// Save how Perry should handle updates from now on, then exit.
20+
///
21+
/// This is the writable half of `[update] mode` in ~/.perry/config.toml —
22+
/// there to save people hand-editing TOML for the one setting they are
23+
/// most likely to want to change.
24+
#[arg(long, value_name = "off|notify|prompt|auto")]
25+
pub mode: Option<String>,
1826
}
1927

2028
pub fn run(
@@ -24,6 +32,10 @@ pub fn run(
2432
verbose: u8,
2533
quiet: bool,
2634
) -> Result<()> {
35+
if let Some(raw) = args.mode.as_deref() {
36+
return set_mode(raw);
37+
}
38+
2739
let current = env!("CARGO_PKG_VERSION");
2840

2941
let status = if !args.force && !update_checker::is_cache_stale() {
@@ -119,3 +131,34 @@ pub fn run(
119131

120132
Ok(())
121133
}
134+
135+
/// Persist `[update] mode`, and nothing else.
136+
///
137+
/// Read-modify-write through the shared loader so the rest of the file — the
138+
/// license key, the telemetry section, anything a newer Perry wrote — comes
139+
/// back out the way it went in.
140+
fn set_mode(raw: &str) -> Result<()> {
141+
let Some(mode) = crate::update_policy::UpdateMode::parse(raw) else {
142+
anyhow::bail!("unknown update mode `{raw}`. Valid values: off, notify, prompt, auto.");
143+
};
144+
if mode == crate::update_policy::UpdateMode::Unknown {
145+
anyhow::bail!("unknown update mode `{raw}`. Valid values: off, notify, prompt, auto.");
146+
}
147+
148+
let mut config = crate::commands::publish::load_config();
149+
config.update.get_or_insert_with(Default::default).mode = Some(mode);
150+
crate::commands::publish::save_config(&config)?;
151+
152+
let path = crate::commands::publish::config_path();
153+
println!("Update mode set to \"{raw}\" ({}).", path.display());
154+
if mode == crate::update_policy::UpdateMode::Auto {
155+
// Say the limits up front rather than letting someone discover them
156+
// the first time an update does not happen.
157+
println!(
158+
"Perry will install updates at the end of a successful run — except \
159+
on a package-manager-managed install, where it names that manager's \
160+
command instead."
161+
);
162+
}
163+
Ok(())
164+
}

0 commit comments

Comments
 (0)