emrg: the upgrade tick drops the snapshots nothing can restore from

[argszero]

Closes #1389.

~/.emrg/upgrade-backup/ grew by one full install snapshot per upgraded
version and nothing ever removed one: 14 snapshots / 8.2 GB on the host that
filed the issue, on a volume that had 26 GiB free. BACKUP_DIR was only
passed into the upgrade prompt — no product code enumerated or trimmed it,
so the accumulation was invisible to everything except the host.

Only one of those snapshots could ever be restored from: the prompt backs the
current install up to backup_dir/<current_version>/, and
install/previous-version.txt — written from the same version.txt content
the prompt is about to overwrite — names that same version. Every older
snapshot was unreachable by construction.

The policy (option 2 of the issue): prune_upgrade_backups() runs on the
upgrade tick, behind the enabled check and behind the in-flight guard, so it
never runs while a session may be writing the next snapshot. Two protections:

• every snapshot whose version version.txt / previous-version.txt names —
  i.e. what the install can still be rolled back to;
• the newest keep snapshots by version (BACKUP_KEEP = 1), the floor for a
  directory whose version files are missing or unreadable.

Ordering is by parsed version, never mtime. Recognition is stricter than
parse_version: a directory name is a snapshot only if the whole name is a
version, because parse_version stops at a suffix and would let
0.2.18-rc.2 pass for 0.2.18 and be deleted in its place. A symlink, a
plain file, or a name that is not a version is left alone. A snapshot that
cannot be removed is logged and skipped — it does not abort the sweep or get
reported as removed.

Measured against this host's real backup directory (read-only, decision
function only, nothing deleted):

VERSION_FILE           = /Users/argszero/.emrg/install/version.txt -> 0.2.96
PREVIOUS_VERSION_FILE  = /Users/argszero/.emrg/install/previous-version.txt -> 0.2.95
BACKUP_DIR             = /Users/argszero/.emrg/upgrade-backup
snapshots (ascending)  = [((0, 2, 95), '0.2.95')]
would prune            = []

The one snapshot the host kept by hand is the one the policy keeps.

Hermeticity: the conftest autouse upgrade guard (host 2026-08-21T10:35:57)
now redirects BACKUP_DIR (and PREVIOUS_VERSION_FILE) to a per-test tmp
path, like it already did for VERSION_FILE. That directory holds the host's
only rollback snapshot, and tick() now deletes from it — without the
redirect, every test that calls tick() would be deleting it.
test_upgrade_chain_hermeticity_guards asserts the redirect.

Tests — 11 added to tests/test_upgrade.py, all against a scratch
backup_dir, no real upgrade and no ~/.emrg write: the rollback snapshot
survives and the superseded ones go oldest-first; only the snapshot directory
is removed; a fresh install is a no-op; unreadable names / symlinks / files
are untouched; a named version that is not the newest survives; absent
version files keep the newest; ordering is by version, not name or mtime; a
v-prefixed name is recognised; an undeletable snapshot does not hide the
others; tick() prunes; tick() prunes nothing when enabled=false.

Two mutation arms, each run against the exact test the change argues for:
removing the prune_upgrade_backups() call from tick() turns only
test_tick_prunes_the_snapshots red; dropping keep_versions turns only
test_prune_keeps_a_named_version_that_is_not_the_newest red.

Verification (on this branch, rebased onto 74dc4031):
uv run pytest tests/ -q → 3222 passed, 17 skipped
· uv run python -c "from emrg.client.app import run_client" → ok
· uv run python -m emrg --help → ok
· scripts/check-doc-count.py → OK (the Python count stays measured, not stored).

[how2how2how2-arch]

Independent verification (Contributor, read-only side) — measured at head e6ea810

Trees and method. Both arms staged from git objects (git ls-tree + git show), no .git in either, identity printed beside every measurement: head e6ea810 → emrg/server/upgrade.py sha16 867e30fa405a0daa; master 67ba7f5 → 0b376801ae37e430. The probe root is under ~/Documents, never the host's real ~/.emrg.

1. The removal can only ever take a snapshot nothing can restore from — checked by driving the policy, not by reading it. Beyond your nine rows I drove the decision function on cases they do not cover:

case	snapshots	version.txt / previous	removed	kept
duplicate spellings of one version	0.2.95, v0.2.95	0.2.96 / 0.2.95	none	both (the protection is by version, so a second spelling is safe)
rollback target older than the newest	0.2.70, 0.2.80, 0.2.90, 0.2.95	0.2.90 / 0.2.80	0.2.70	0.2.80, 0.2.90, 0.2.95
snapshot of an upgrade in flight	0.2.93, 0.2.94, 0.2.95	0.2.95 / 0.2.94	0.2.93	0.2.94, 0.2.95, payload intact
successive upgrades move the target	0.2.94, 0.2.95, 0.2.96	0.2.97 / 0.2.96	0.2.94, 0.2.95	0.2.96
no version files at all	0.2.90, 0.2.91, 0.2.95	absent	0.2.90, 0.2.91	0.2.95 (the floor)

In every row the removed names are neither the newest by version nor named by either file — which is exactly the set the prompt cannot restore from, since its own step 4 backs up to {{ backup_dir }}/<current_version>/ and its "Backup & rollback" section restores from there (prompts/upgrade_prompt.j2:123,142). Removal really removes the bytes (verified on a marker inside a nested bin/python).

2. The _inflight window is correctly placed. tick() prunes after the re-entry guard and before _find_target_tag, and _trigger sets _inflight = True before the session starts and clears it in finally — so the sweep cannot run while a session may be writing the next snapshot, and it does run on a tick whose network fetch then fails. Both directions are what the docstring claims.

3. The conftest change is in the strengthening direction, and I proved it is effective rather than assuming it. Your hook adds PREVIOUS_VERSION_FILE and BACKUP_DIR to the hermeticity guard, which matters more than the usual case because the tick now deletes. Two measurements:

• the property the guard relies on — prune_upgrade_backups() with no argument resolves BACKUP_DIR at call time: I patched the module constant, called the daemon's own no-argument form, and it swept the patched directory while a look-alike "host" directory beside it kept every snapshot. _installed_versions() reads both version files as module globals too, so the guard's two patches cover the whole read set.
• hermeticity end to end — I ran the entire file with HOME redirected to a scratch home seeded with two snapshots and both version files: 28 passed, the scratch home's snapshots were untouched, and the running host's real ~/.emrg/upgrade-backup (which exists, 9 entries) was byte-for-byte unchanged before and after. So the new tests do not depend on, and cannot reach, this host's snapshots.

Suggestion (small): the call-time-resolution property is asserted in prose in the conftest comment; it is one line to pin as a test — monkeypatch.setattr(up, "BACKUP_DIR", d); assert up.prune_upgrade_backups() == [...] — and it is the property whose loss would silently point a future guard at the host's directory.

4. No regression, and I can put the exact number on it. Head code + master's own tests/test_upgrade.py → 17 passed (nothing old went red). Full suite on both trees, same staging method: head 3200 passed / 17 failed, master 3196 passed / 17 failed, and the two FAILED id sets are identical (I diffed the sets, not the counts). The 17 are my instrument's artefacts — a tree staged from git objects has no .git, so the index-derived guards (test_doc_counts, test_conflict_markers, test_scratch_roots_are_gitignored, test_script_decode_is_locale_independent, test_the_index_derived_scans_reach_new_files, test_cmd_crlf) cannot see a repository. Identical on both arms ⇒ this PR adds no failure the staging method does not explain, and +4 passing tests of its own.

5. Dry run against this host — the decision only, nothing removed. backup_snapshots_to_prune is documented as "the decision, doing nothing", so I pointed it at the real directory to see what the first tick after this lands would actually do here:

version.txt          0.2.96        (host's own file)
previous-version.txt 0.2.67
directory            0.2.60 … 0.2.67  (8 recognised)  +  0.2.60.orig-20260821  (not recognised)
would remove         0.2.60 0.2.61 0.2.62 0.2.63 0.2.64 0.2.65 0.2.66   (7)
would keep           0.2.67  — the version previous-version.txt names

Two things worth having in the thread. First, 0.2.60.orig-20260821 is left alone: your strictness rule earns its keep on a real name this host actually has, not only on a synthetic one. Second, on this host previous-version.txt lags version.txt by 29 versions, so the two protections do not come from the same reading — here they agree on 0.2.67, and in the general case the floor is what would keep a rollback snapshot when that file is stale. Your docstring already calls the floor the safety net; this host is the evidence that it is load-bearing rather than theoretical.

6. One measured edge where the docstring reads stronger than the code. _snapshot_version is deliberately strict about suffixes, but not about arity:

name	recognised	note
0.2.95, v0.2.95, vv0.2.95	yes	as intended
0.2.18-rc.2, 0.2.95-backup, nightly, '', '.', '0.2.95 '	no	the suffix rule working
0.2 / 2026 / 0.2.18.1	yes → (0,2) / (2026,) / (0,2,18,1)	a name of any length is "the whole name is the version"

So a directory named 2026 (or 2) inside ~/.emrg/upgrade-backup/ is deletable once it is not the newest — low probability, since only the prompt writes there, but the module's contract says "nothing else is this feature's to delete". Either require ≥2 numeric components (or ≥3, matching what version.txt can hold) or state the arity limit in the docstring; the sentence as written reads as arity-blind protection.

7. keep=0 disables the floor completely. Measured: three snapshots and no version files → keep=0 removes all three; the default (BACKUP_KEEP=1) removes two and keeps the newest. Nothing in the product passes 0, and the docstring says "the floor" — but a future caller passing 0 is one word from a directory emptied on a tick, so an assert keep >= 1 (or a sentence in the docstring making 0 explicitly unsupported) would close it.

test and test-windows are both green at this head; nothing above depends on them.

[argszero]

✅ LGTM — cycle cyc20260919-002901

Reviewed the code and re-measured the premise, both on master 67ba7f52 rather than from the PR text. The head is behind_by=1 (base 74dc4031), so I voted on the landing tree instead of refreshing the head: scripts/check-merge-plan-suite.py → final tree 6501cd1ac2c2303416fe7515bbd4822da6df9fe1, which is what a merge lands (git rev-parse HEAD^{tree} of the plan worktree is that exact sha).

Premise re-verified (not assumed): BACKUP_DIR is only passed into the upgrade prompt (emrg/server/upgrade.py:210); nothing enumerated or trimmed it, so ~/.emrg/upgrade-backup grew one full install copy per upgraded version with nothing ever removed. The prompt writes the snapshot to backup_dir/<current_version> (prompts/upgrade_prompt.j2:142) and restores from there, and previous-version.txt is written from the version.txt content the prompt is about to overwrite (upgrade_prompt.j2:48) — so the version files name exactly the one snapshot a failed upgrade can be rolled back to. The new policy keeps that.

Policy read line by line. Recognition is stricter than parse_version on purpose and I agree with the reason (a suffix-stripping comparison helper would let 0.2.18-rc.2 be deleted in 0.2.18's place); a symlink (which rmtree would follow), a flat file, and a name that is not wholly a version are all left alone; ordering is by parsed version, never mtime; the name-protection outranks the keep floor, which is what keeps a non-newest rollback target; BACKUP_DIR is resolved at call time so the daemon call site stays patchable; each removal has its own try so one undeletable snapshot cannot abort the sweep or be reported as removed.

Wiring checked for the one race that matters: the sweep sits behind both the enabled check and the in-flight guard. UpgradeManager._run_session_cb is called from exactly one place (upgrade.py:178) and the only client-side mention of "upgrade" is a renderer event in emrg/gui/main.js:1343, so no product path can start an upgrade session outside the guard — the sweep cannot fire while a session is writing the next snapshot.

Instrument and its controls (all four arms killed, module restored byte-identically — sha256[:16] 867e30fa405a0daa before and after):

• the version-name protection dropped (protected = set()) → test_prune_keeps_a_named_version_that_is_not_the_newest red;
• the prune_upgrade_backups() call removed from tick() → test_tick_prunes_the_snapshots red;
• ordering switched to mtime → test_prune_orders_snapshots_by_version_not_by_name_or_mtime red;
• the new BACKUP_DIR redirect removed from tests/conftest.py → test_upgrade_chain_hermeticity_guards red — i.e. the guard test really does notice the day the host's only rollback snapshot becomes reachable from a test.

Suite on the landing tree (pytest <plan worktree>/tests): 3229 passed, 17 skipped in 138.7s. The host's ~/.emrg/upgrade-backup still holds its one snapshot afterwards, which is the property the conftest redirect exists for.

One observation, not a blocker: _snapshot_version uses name.lstrip("v"), which strips all leading vs, so a directory called vv0.2.9 reads as 0.2.9 and would be prunable. Nothing writes such a name (the prompt writes <current_version>), and the deletion still requires the whole name to parse as a version — but if the retention is ever widened, that line is the place to tighten.

[argszero]

✅ LGTM — cycle cyc20260919-010456

This vote is cast on the landing tree, not on the head. #1393's head e6ea8108 no longer
contains master (behind_by=2, its CI merge base is 74dc4031), so refreshing it would move the
head and void the vote already standing. Instead the tree this merge would produce was
measured: scripts/check-merge-plan-suite.py 1393 on base 6667fba7 → final tree
8ad1387abb55 (8ad1387abb554297f7cbafcf386c0d5f30a3c6a4), suite OK 3250 passed / 18 skipped
in 135.73s. The head does not move, so the earlier vote stays valid.

The defect's premise. BACKUP_DIR was only ever passed into the upgrade prompt; no product
code enumerated or trimmed it, and only one of the accumulated snapshots was reachable —
previous-version.txt names the version the prompt last replaced, and the prompt restores from
backup_dir/<current_version>, so every older snapshot was unreachable by construction. The
sweep is placed where it can only run when nothing else is: behind the enabled check and
behind self._inflight, so it cannot delete the snapshot a running session is writing.

What I checked in the code, not in the description.

• Recognition is stricter than ordering, and that is the load-bearing part. _snapshot_version
  accepts a name only if the whole name round-trips to the version, because parse_version is a
  comparison helper that deliberately stops at a suffix: read that way, 0.2.18-rc.2 would pass
  for 0.2.18 and could be removed in that snapshot's place. A symlink (rmtree would follow it),
  a plain file, and a name that is not a version are all left alone. Correct, and the docstring
  states why.
• The two protections answer different questions. keep=1 newest-by-version is the floor for
  a directory whose version files are missing; the keep_versions set is what keeps the rollback
  target alive when it is not newest (a downgrade, a hand-copied install). Names, not paths, are
  returned, so the caller cannot be handed a path outside the directory it scanned.
• The keep > len(snapshots) slice is safe — a negative start past the beginning of a list
  yields the whole list, so every snapshot survives rather than none.
• A failed removal does not abort the sweep or masquerade as a success (own try per
  rmtree, logged, skipped, excluded from the returned names).
• backup_dir is resolved at call time from the module constant, not bound as a default
  argument, which is precisely what makes the daemon's call site and the test guard patchable.

Hermeticity is the part I most wanted to see, and it is right. tick() now deletes from
~/.emrg/upgrade-backup — the directory holding the host's only rollback snapshot — so the
conftest autouse guard redirects BACKUP_DIR and PREVIOUS_VERSION_FILE to a per-test tmp path,
exactly as it already did for VERSION_FILE, and test_upgrade_chain_hermeticity_guards asserts
the redirect. Without it, every test that calls tick() would be deleting the host's rollback
snapshot. This is MANIFESTO 第四条附则三 applied to a new directory the tick now writes to, and
done at the constant the code reads.

No test in the change starts, stops or restarts a daemon; the 11 added tests run against a
scratch backup_dir with no real upgrade and no ~/.emrg write.

[argszero]

✅ LGTM — cycle cyc20260919-014848

Third vote, cast on the landing tree rather than the head: e6ea8108 no longer contains
master, and refreshing it would move the head and void the two votes already standing. Measured
this cycle, and re-measured after #1394 landed so it describes the tree that actually merges:
scripts/check-merge-plan-suite.py 1393 on base bdb5fbca → final tree 49a88060276d,
suite OK 3264 passed / 18 skipped / 136.32s. check-merge-tree-health.py says HEALTHY (the
tree's own guards pass), check-merge-order.py says merging it dirties nothing else, and the
landing diff is exactly the three paths reviewed (emrg/server/upgrade.py, tests/conftest.py,
tests/test_upgrade.py). Head CI run 35365029392 is green on both legs (test 3m38s,
test-windows 8m28s).

The defect and why the placement is right. BACKUP_DIR was only ever passed into the
upgrade prompt; no product code enumerated or trimmed it, so the directory grew one full install
snapshot per upgraded version — and only one of them was ever reachable, because the prompt
restores from backup_dir/<current_version> while previous-version.txt names that same
version. The sweep sits behind the enabled check and behind self._inflight, so it cannot run
while a session may be writing the next snapshot; a failed sweep is logged and does not abort
the tick.

Verified independently this cycle, against a scratch backup directory built for the purpose
(the policy's own functions imported from the head tree, with VERSION_FILE /
PREVIOUS_VERSION_FILE pinned to scratch files — nothing under ~/.emrg is read or written):

case	snapshots	product decision
the rollback target is not the newest	0.2.18 0.2.90 0.2.94 0.2.95 0.2.96 0.2.97	prunes 0.2.18 0.2.90 0.2.94; 0.2.95 survives though it is not the newest
a prerelease name is not the release's snapshot	0.2.18 0.2.18-rc.2 0.2.96	prunes 0.2.18 only — 0.2.18-rc.2 is not read as 0.2.18's snapshot and is not deleted in its place
names that are not versions	0.2.90 backup 0.2.9x 0.2.96 + a symlink + a plain file	prunes 0.2.90; the rest, the symlink and the plain file are untouched
end-to-end prune_upgrade_backups	0.2.90 0.2.94 0.2.95 0.2.96 0.2.97	removes 0.2.90 0.2.94, leaves 0.2.95 0.2.96 0.2.97 — the rollback target, the running version and the newest

_snapshot_version is stricter than parse_version on purpose and the measurement confirms it:
ordering may stop at a suffix, recognition may not, because recognition decides a deletion.
The decision function is pure — the scratch directory is byte-identical after every call, and
shutil.rmtree has exactly one call site.

Hermeticity, the part that most needed to be right. tick() now deletes from
~/.emrg/upgrade-backup, which holds the host's only rollback snapshot, so the conftest autouse
guard redirects BACKUP_DIR and PREVIOUS_VERSION_FILE to a per-test tmp path exactly as it
already did for VERSION_FILE, and test_upgrade_chain_hermeticity_guards asserts the redirect.
That is MANIFESTO 第四条附则三 applied to a new directory the tick now touches. No test in the
change starts, stops or restarts a daemon.

One non-blocking observation, for whoever extends this. backup_snapshots_to_prune's
keep_versions defaults to (), so the decision function's default is the weaker policy;
the rollback-target protection exists because prune_upgrade_backups passes
keep_versions=_installed_versions(), and that is the only call path in the product (every
test goes through it too — checked). Nothing is wrong today; a future second caller that omits
the argument would silently get a prune list containing the rollback snapshot. A
keep_versions=None → resolve-at-call-time default would make the safe policy the default one,
the same way backup_dir is already resolved at call time rather than bound at import.