Skip to content

Commit fcbe347

Browse files
committed
feat(cli): a release cooldown for auto, a skip-this-version answer, and docs
auto now waits min_age_hours (default 24) before installing a release; notify and prompt are unaffected because they tell a human who can decide. A release published by mistake, pulled soon after, or published by someone who should not have been able to is most dangerous in its first hours, and waiting means this machine is not the one that finds out. An UNKNOWN publish date counts as too fresh rather than old enough. The abbreviated npm packument carries no dates, so the other choice would switch the cooldown off for exactly the users on the cheapest source -- present in the config, absent in effect. min_age_hours = 0 disables it deliberately. The prompt gains a third answer. 'No' and 'never tell me about this one' are different intentions, and with two answers someone who dislikes one release has to switch the mode off, which then hides the release that fixes it. The cache records its schema and a foreign value -- including an absent one -- is discarded rather than migrated. Keeping every field optional forever so older shapes load buys one saved request for a CACHE in exchange for Option fields that only describe versions nobody runs. Check sources lose their aliases for the same reason. Docs: a new cli/updates.md covering the default, every [update] key, the modes and their refusals, the four sources, the cooldown, skipping, what a check transmits and where state lives; commands.md rewritten around --mode; installation.md gains the per-package-manager upgrade table. Carries #7787, #7784 and #7785 as its base; those collapse out as they merge.
1 parent 0a2bf15 commit fcbe347

17 files changed

Lines changed: 2267 additions & 105 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: 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: 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: 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: 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(),

0 commit comments

Comments
 (0)