Skip to content

Commit 373f3fd

Browse files
committed
feat(cli): hold back brand-new releases, remember a skipped version, and document the whole surface
Three finishing touches on top of the update ladder. A release younger than the cooldown window is never installed automatically, so a version that is published and then pulled minutes later cannot land on anyone's machine. The prompt gains a third answer — skip this version — which silences that one release without silencing the next. And the notice carries the release headline when the source already returned it, at no extra request. Adds docs/src/cli/updates.md as the one page describing the settings, the environment variables that override them, and what a check actually sends.
1 parent b9415d7 commit 373f3fd

19 files changed

Lines changed: 2639 additions & 128 deletions
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>
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
### Added
2+
3+
**Where Perry asks "what is the latest version?" is now a choice.** It used to
4+
walk one fixed list — an override, the config, Perry Hub, then the GitHub
5+
releases API — and read a GitHub-releases-shaped document from whichever
6+
answered first. That is fine while everyone installs the same way, and wrong as
7+
soon as they do not: an npm user's "latest" is whatever the registry's `latest`
8+
dist-tag says, and asking GitHub instead can announce a version their package
9+
manager cannot install yet.
10+
11+
```toml
12+
[update]
13+
source = "npm" # gh-releases | npm | gh-registry | custom
14+
package = "@perryts/perry" # npm-shaped sources; defaults to Perry's own
15+
registry = "..." # npm-shaped sources; defaults to the public registry
16+
server = "..." # the URL for `custom`, and the mirror override
17+
```
18+
19+
Unset keeps the historical ladder, so nothing changes for anyone who does not
20+
set it — except on an **npm-managed install**, which now defaults to asking npm,
21+
because that is the version its own package manager can actually install.
22+
23+
<details>
24+
<summary><b>The split that matters: checking is not downloading</b></summary>
25+
26+
A check source answers one question and returns a version, a link, a publish
27+
time and a headline. It does **not** decide where the binary comes from.
28+
Artifacts and their signed manifest always resolve from the release
29+
infrastructure, whatever the check source is.
30+
31+
That separation is load-bearing rather than tidy. The manifest — Ed25519 over
32+
the artifact's digest and version — is what makes a self-update trustworthy,
33+
and a check source is a URL a user can point anywhere. Letting it redirect the
34+
download would turn a configuration setting into a way to install an arbitrary
35+
binary. Whoever answers "what is new?" never gets to answer "what should I
36+
run?", and there is a test that fails if a source ever leaks into the artifact
37+
ladder.
38+
39+
The old `get_update_servers` and its private config reader are **deleted**
40+
rather than left beside the new code, so the compiler enforces that both call
41+
sites moved. A new abstraction with the old ladder still wired up underneath is
42+
the shape where four sources exist, pass their own tests, and are never
43+
reached.
44+
</details>
45+
46+
<details>
47+
<summary><b>Credentials go to exactly one of the four</b></summary>
48+
49+
The npm shapes ask for the *abbreviated* packument
50+
(`Accept: application/vnd.npm.install-v1+json`) — smaller, cacheable, and the
51+
document npm itself requests for this question. It also avoids GitHub's
52+
unauthenticated API rate limit, which the old ladder shared with everything
53+
else on the machine.
54+
55+
The public registry is asked **without credentials**, and a test asserts no
56+
`Authorization` header is sent: a token there would be a leak, not a
57+
convenience. GitHub Packages does need one, so that shape reads `GH_TOKEN` /
58+
`GITHUB_TOKEN` and fails with a sentence naming the fix when neither is set,
59+
rather than retrying anonymously and reporting the resulting 404 as "up to
60+
date".
61+
62+
A configured source does not fall back to the ladder when it errors. Somebody
63+
who said "ask npm" and got a failure wants to hear that, not a version from
64+
somewhere they never named.
65+
</details>
66+
67+
<details>
68+
<summary><b>Tests</b></summary>
69+
70+
11 new, all parsing real response shapes from string fixtures so no network is
71+
involved:
72+
73+
- a GitHub release document, including that the `v` prefix is stripped;
74+
- an abbreviated packument, which has no `time` map — so the publish date reads
75+
"unknown" rather than being invented, which matters because the release
76+
cooldown in the next slice depends on it;
77+
- a full packument, which does supply it;
78+
- a custom manifest with only a `version`, and one with every optional field;
79+
- that each shape **rejects the others' documents** rather than reading a field
80+
that happens to be present — a registry answering a gh-releases request must
81+
be an error, not a version of `""`;
82+
- that a scoped package's `/` is percent-encoded, or the registry reads the
83+
scope as a path segment and answers 404;
84+
- that an unknown `source` name falls back instead of failing, so a config
85+
written by a newer Perry does not break an older one;
86+
- that `custom` with no URL is treated as a missing key rather than a default;
87+
- that an npm install defaults to npm and every other channel keeps the ladder;
88+
- that no check source can reach the artifact ladder;
89+
- and both credential rules.
90+
91+
`cargo test -p perry`: 925 passed, 0 failed.
92+
</details>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
### Changed
2+
3+
**The update cache carries its shape, and a foreign shape is discarded rather
4+
than migrated.** `~/.perry/update-check.json` now records a `schema` number; a
5+
value this build does not recognize — including its absence, which reads as `0`
6+
means the file is thrown away and the next check rewrites it.
7+
8+
This replaces the alternative, which was to keep every field optional forever so
9+
that older shapes still load. That trade is a bad one for a cache: it buys one
10+
saved network request in exchange for a set of `Option` fields that only exist
11+
to describe versions nobody runs, and that nothing ever removes. Bumping
12+
`CACHE_SCHEMA` is now the whole migration story.
13+
14+
**One spelling per check source.** `github`, `npm-registry` and
15+
`github-packages` are gone; the names are `gh-releases`, `npm`, `gh-registry`
16+
and `custom`. A set of accepted aliases is a surface to document and test
17+
forever in exchange for saving one look at the docs.
18+
19+
An unknown `source` still falls back to the default rather than failing, but for
20+
a different reason than compatibility: an update check is the wrong place to
21+
turn a config typo into a hard error.
22+
23+
<details>
24+
<summary><b>Tests</b></summary>
25+
26+
The test that asserted a pre-throttle cache still loads is replaced by one
27+
asserting the opposite — that a foreign schema, and an absent one, are both
28+
recognized as not-ours. Verified at runtime as well: a planted cache with no
29+
schema, claiming version `99.0.0` and a `last_check` in 2099, was ignored, a
30+
real check ran, and the file came back stamped `"schema": 1`.
31+
32+
`cargo test -p perry`: 930 passed, 0 failed.
33+
</details>
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
### Added
2+
3+
**`auto` waits a day before installing a brand-new release.** `min_age_hours`
4+
defaults to 24 for `auto` and 0 for every other mode, so a notice still mentions
5+
a release the moment it exists while an unattended install holds off.
6+
7+
A release published by mistake, pulled shortly after, or published by someone
8+
who should not have been able to is most dangerous in its first hours. Waiting
9+
costs nothing and means your machine is not the one that finds out. `notify` and
10+
`prompt` are unaffected — they tell a human, who can decide.
11+
12+
**An unknown publish date counts as too fresh, not as old enough.** The
13+
abbreviated npm document carries no dates, so treating unknown as "old enough"
14+
would switch the cooldown off for exactly the people using the cheapest source —
15+
a protection present in the config and absent in effect. `min_age_hours = 0`
16+
turns it off deliberately.
17+
18+
**The prompt has three answers.** "No" and "never tell me about this one" are
19+
different intentions, and with only two answers a user who does not want one
20+
specific release has to switch the whole mode off — which then hides the release
21+
that fixes it. Answering the third writes `skip_version`, which suppresses
22+
exactly that version; the next release is mentioned normally.
23+
24+
**The notice says what the release is,** not only that one exists. Sources that
25+
carry a title now pass it through, and it prints under the version line at no
26+
extra request.
27+
28+
### Documentation
29+
30+
New page `docs/src/cli/updates.md`, covering the default behaviour, everything
31+
in `[update]`, the four modes and their three refusals, the four check sources,
32+
the cooldown, skipping a version, exactly what a check transmits, and where the
33+
two files live. `perry update`'s section in `commands.md` is rewritten around
34+
`--mode`; `installation.md` gains the per-package-manager upgrade table; the
35+
environment tables in `flags.md` gain `NO_UPDATE_NOTIFIER` and
36+
`PERRY_UPDATE_MODE`.
37+
38+
<details>
39+
<summary><b>Tests</b></summary>
40+
41+
Four new, bringing the update surface to 22 in `update_policy`:
42+
43+
- `auto` holds off inside the cooldown and installs past it, while `notify` and
44+
`prompt` are never held back;
45+
- an unknown release age is treated as too fresh, and an explicit `0` still lets
46+
it through so nobody is stuck;
47+
- the cooldown defaults to a day for `auto` only, and an explicit value wins for
48+
every mode;
49+
- a skipped version suppresses itself and **not** the next one, which is what
50+
separates it from switching notices off.
51+
52+
`cargo test -p perry`: 929 passed, 0 failed.
53+
</details>
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
### Fixed
2+
3+
Five defects in the update surface, all found in review of
4+
[#7749](https://github.com/PerryTS/perry/pull/7749) after it had merged.
5+
6+
**The config warning escaped the rules that were meant to silence it.** The
7+
"unrecognized `[update] mode`" line was printed inside `UpdatePolicy::resolve`,
8+
before the precedence rules it sits behind had been applied — so it reached
9+
stderr during `--format json`, in CI, with a piped stderr, and under `--quiet`.
10+
Those rules exist to keep exactly those runs silent, and the one line whose job
11+
was to report a config problem was the one line ignoring them. It is now held on
12+
the policy and emitted at the single point where the run is known to be speaking
13+
at all.
14+
15+
**The notify interval throttled on time alone, so it swallowed the next
16+
release.** The documented contract is that the interval throttles repeats of
17+
*the same* update. Keyed only on a timestamp, it also suppressed a **different**
18+
version that arrived inside the window — so somebody setting a week-long
19+
interval to stop being nagged about one release would also have been denied the
20+
release that fixed it. The cache now records which version it announced, and a
21+
different version is announced regardless of the interval.
22+
23+
**The interval comparison was signed.** `Duration::as_secs() as i64` goes
24+
negative for a large enough configured value, and a negative interval reads as
25+
already-elapsed — so an absurd value would have notified on *every* run instead
26+
of suppressing. The comparison is unsigned.
27+
28+
**Two `perry` processes could corrupt the cache.** Every write used one shared
29+
`*.json.tmp`, so two writers each wrote it and each renamed it: the loser's
30+
rename landed a file the winner was still writing into. Each write now builds
31+
its own temporary name.
32+
33+
**A refresh could erase a notice recorded while its request was in flight.**
34+
`fetch_latest_version` read the notice state *before* issuing its request and
35+
wrote it back afterwards, overwriting anything recorded in between — telling the
36+
user about the same release twice. The read-modify-write pairs are now
37+
serialized by a lock file, and the refresh re-reads inside that lock immediately
38+
before replacing.
39+
40+
<details>
41+
<summary><b>Tests</b></summary>
42+
43+
Two new contract tests, both sabotage-verified — reverting either fix turns its
44+
test red:
45+
46+
- a different version is announced regardless of the interval, and never having
47+
announced anything counts as "not this version";
48+
- an enormous interval still suppresses rather than wrapping into notifying.
49+
50+
The existing interval tests are unchanged in intent: they now go through a
51+
helper that holds the announced version constant, so they still exercise only
52+
the interval arithmetic.
53+
54+
`cargo test -p perry`: 904 passed, 0 failed.
55+
</details>

crates/perry/src/commands/doctor.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,16 +286,41 @@ 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+
// Configured, not effective: `doctor --format json` and `doctor | less` both
295+
// suppress the update surface for their own run, so reporting the effective
296+
// mode would answer "off" to the very question being asked.
297+
let mut detail = format!(" (mode: {}", policy.configured_mode.label());
298+
if !policy.is_active() && policy.configured_mode != crate::update_policy::UpdateMode::Off {
299+
detail.push_str("; suppressed for this run");
300+
}
301+
if let Some(command) = channel.upgrade_command() {
302+
detail.push_str(&format!(
303+
"; installed by {} — upgrade with `{}`",
304+
channel.label(),
305+
command
306+
));
307+
}
308+
detail.push(')');
309+
let context = detail;
310+
289311
match update_checker::check_cached_status() {
290312
update_checker::UpdateStatus::UpdateAvailable { latest, .. } => CheckResult {
291313
name: "update status".to_string(),
292314
status: CheckStatus::Warning,
293-
details: Some(format!("v{} available — run `perry update`", latest)),
315+
details: Some(format!(
316+
"v{} available — run `perry update`{}",
317+
latest, context
318+
)),
294319
},
295320
update_checker::UpdateStatus::UpToDate => CheckResult {
296321
name: "update status".to_string(),
297322
status: CheckStatus::Ok,
298-
details: Some("up to date".to_string()),
323+
details: Some(format!("up to date{}", context)),
299324
},
300325
update_checker::UpdateStatus::CheckFailed => CheckResult {
301326
name: "update status".to_string(),

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ pub use args::PublishArgs;
3535
#[cfg(test)]
3636
pub(crate) use saved_config::IosSavedConfig; // consumed only by tests
3737
pub(crate) use saved_config::{
38-
check_beta_consent, config_path, is_interactive, load_config, prompt_input, report_beta_error,
38+
check_beta_consent, config_path, is_interactive, load_config, load_config_checked,
39+
prompt_input, report_beta_error, update_config_file,
3940
save_config, AndroidSavedConfig, AppleSavedConfig, HarmonyosSavedConfig, PerryConfig,
4041
};
4142
pub(crate) use tarball::create_project_tarball_with_filters;

0 commit comments

Comments
 (0)