v0.3.1: automated releases + version-drift check

The "Automated releases" entry was written under v0.3.0, but that commit landed
after the v0.3.0 tag -- the CHANGELOG was claiming the release contained something
it did not. Moved to its own version rather than left as a quiet inaccuracy.

Bumps VERSION to 0.3.1 across all four scripts, which the new consistency check
now enforces.

.github/workflows/ci.yml

@@ -51,7 +51,7 @@ jobs:
         run: python -m pip install --upgrade pip pytest ruff
 
       - name: ruff
-        run: ruff check patch tests
+        run: ruff check patch tests tools
 
       - name: pytest
         run: pytest tests -v

.github/workflows/release.yml

@@ -0,0 +1,102 @@
+name: Release
+
+# Push a tag, get a release. The body always comes from CHANGELOG.md, so there is
+# no second place to write release notes and therefore no second place for them to
+# go stale.
+#
+#   git tag -a v0.4.0 -m "v0.4.0" && git push origin v0.4.0
+#
+# workflow_dispatch re-cuts (or updates) the release for a tag that already
+# exists, since re-pushing an existing tag triggers nothing.
+#
+# It checks out the TAG, because the tagged code is what people install and it has
+# to pass its own tests. That means it only works for tags that actually contain
+# this tooling (>= v0.3.0). Tags older than that were backfilled by hand.
+
+on:
+  push:
+    tags: ["v*"]
+  workflow_dispatch:
+    inputs:
+      tag:
+        description: "Existing tag to create a release for (e.g. v0.2.1)"
+        required: true
+        type: string
+
+permissions:
+  contents: write
+
+jobs:
+  release:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Resolve tag
+        id: tag
+        run: |
+          if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
+            echo "tag=${{ inputs.tag }}" >> "$GITHUB_OUTPUT"
+          else
+            echo "tag=${GITHUB_REF#refs/tags/}" >> "$GITHUB_OUTPUT"
+          fi
+
+      - uses: actions/checkout@v4
+        with:
+          ref: ${{ steps.tag.outputs.tag }}
+          fetch-depth: 0
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.13"
+
+      # Never publish a release for code that does not pass its own tests. A
+      # tagged commit is what people install; it has to be at least as good as
+      # main.
+      - name: install dev deps
+        run: python -m pip install --upgrade pip pytest ruff
+
+      - name: ruff
+        run: ruff check patch tests tools
+
+      - name: pytest
+        run: pytest tests -q
+
+      - name: shell syntax
+        run: |
+          fail=0
+          while IFS= read -r f; do
+            bash -n "$f" || { echo "::error file=$f::bash syntax error"; fail=1; }
+          done < <(find . -name '*.sh' -not -path './.git/*')
+          exit $fail
+
+      # Catches the failure mode this repo actually had: VERSION= drifted to
+      # three different values across the scripts, and nothing noticed.
+      - name: version matches tag and CHANGELOG has a section
+        run: python3 tools/release_notes.py check "${{ steps.tag.outputs.tag }}"
+
+      - name: extract release notes from CHANGELOG
+        run: |
+          python3 tools/release_notes.py notes "${{ steps.tag.outputs.tag }}" > /tmp/notes.md
+          echo "--- release body ---"
+          cat /tmp/notes.md
+
+      - name: create or update the release
+        env:
+          GH_TOKEN: ${{ github.token }}
+          TAG: ${{ steps.tag.outputs.tag }}
+        run: |
+          # Pre-1.0 and any -rc/-beta suffix ship as prereleases, not "Latest".
+          prerelease=""
+          case "$TAG" in
+            *-rc*|*-beta*|*-alpha*) prerelease="--prerelease" ;;
+          esac
+
+          if gh release view "$TAG" >/dev/null 2>&1; then
+            echo "Release $TAG exists — updating notes."
+            gh release edit "$TAG" --notes-file /tmp/notes.md
+          else
+            # shellcheck disable=SC2086
+            gh release create "$TAG" \
+              --title "$TAG" \
+              --notes-file /tmp/notes.md \
+              $prerelease
+          fi

CHANGELOG.md

@@ -1,6 +1,27 @@
 # Changelog
 
-## v0.3.0 — 2026-07-12
+## v0.3.1 — 2026-07-13
+
+### Added
+
+- **Automated releases.** Pushing a `v*` tag runs the full test suite and then
+  cuts a GitHub release whose body is the matching `CHANGELOG.md` section — so
+  release notes have exactly one source of truth, and no second place to go stale.
+  The workflow refuses to publish if the tests fail, if the tag does not match the
+  `VERSION=` declared by every script, or if the CHANGELOG has no section for it.
+
+- **Version-drift check.** `VERSION=` had silently diverged to three different
+  values across `install.sh`, `uninstall.sh`, `recover.sh`, and `patch/apply.sh`,
+  and nothing noticed. CI now asserts every script agrees with the others and with
+  the newest CHANGELOG entry.
+
+### Note
+
+- Releases for `v0.2.0` and `v0.2.1` were backfilled — they had been tagged but
+  never released, so the releases page jumped v0.1.0 → v0.3.0 and hid the fix for
+  the boot race that took every app down.
+
+## v0.3.0 — 2026-07-13
 
 ### Added
 
@@ -165,6 +186,17 @@
   middlewared; there is now a regression test that executes apply.sh's own probe
   code against the real wrapped source.
 
+### Changed (production audit)
+
+- **`delete_snapshot_tree` now uses a single recursive delete.** It previously
+  removed the parent and each child snapshot one at a time — 252 sequential
+  middleware calls on a real pool. That is slow, but the real problem is that it
+  is **not atomic**: a run killed part-way through the sweep leaves exactly the
+  orphaned snapshots the function exists to prevent. It now issues one
+  `zfs.snapshot.delete(..., {"recursive": True})` and falls back to the
+  name-by-name sweep only when that fails (e.g. stock's `finally` already removed
+  the parent, which leaves the children behind).
+
 ### Refactored
 
 - Staging teardown had been copy-pasted into `uninstall.sh` and `recover.sh` —
@@ -176,6 +208,17 @@
   middlewared-restart case (which empties it) that must not orphan a snapshot
   tree. One record, on disk, or none.
 
+### Validated in production
+
+An unattended scheduled backup of a live 252-dataset pool (`/mnt/Tap`, TrueNAS
+25.10) ran through the staging tree end to end:
+
+- 252 datasets recursively snapshotted, 173 bind mounts built and verified
+- completed in **18m14s**, `SUCCESS` — the same task previously stalled at 74%
+  for over 12 hours reading live files
+- **zero** orphaned ZFS snapshots and **zero** stale mounts afterwards, which is
+  the failure mode that would otherwise have accumulated 251 snapshots per run
+
 ### Known issues
 
 - Stock `restic_backup()` deletes the ZFS snapshot in its own `finally`, which

README.md

@@ -38,8 +38,8 @@ knowing:
   log after an update.
 - If you file a TrueNAS bug report, **remove the patch first** and reproduce on a
   stock system.
-- **Test your restores.** That is true of any backup, but it matters more here —
-  see [Verifying it works](#verifying-it-works).
+- **Test your restores.** True of any backup, but it matters more here — see
+  [Verifying it works](#verifying-it-works).
 - Provided as-is, no warranty. See LICENSE.
 
 The patch is two independent modules — **providers** (B2/S3) and **nested**
@@ -112,10 +112,14 @@ bash install.sh --disable-nested-snapshots
 With neither flag `install.sh` leaves the setting alone, so `git pull && bash
 install.sh` won't flip it. The providers module is unaffected either way.
 
-The planner and the snapshot lifecycle have been validated against a real
-250-dataset pool. The `mount --bind` staging step has not yet been exercised by a
-live backup run, so confirm your first backup actually contains child-dataset
-data before relying on it — see [Verifying it works](#verifying-it-works).
+Validated end to end on a live 252-dataset pool: an unattended scheduled backup
+of `/mnt/Tap` built a 173-mount staging tree, completed in **18m14s**, and left
+**zero** orphaned snapshots and **zero** stale mounts behind. The same backup
+previously stalled at 74% for over 12 hours reading live files.
+
+Still: verify your own first run actually contains child-dataset data before you
+rely on it — see [Verifying it works](#verifying-it-works). That advice is not
+boilerplate; it is the specific thing this feature exists to make true.
 
 TrueCloud Backup's **Take Snapshot** option makes restic read from a frozen ZFS
 snapshot instead of live files. Without it the backup reads data *while apps are
@@ -248,6 +252,7 @@ mount | grep truecloud-nested                            # expect no output
 | Backup fails: `dataset '…' has no snapshot '…'; refusing to back up an incomplete tree` | Working as designed — a descendant dataset was not covered by the snapshot. The backup is refused rather than silently omitting that data. |
 | Backup fails: `snapshot '…' cannot be read (Permission denied)` | The snapshot exists but is unreadable. Middleware runs as root, so this indicates a real permissions problem, not a missing snapshot. |
 | `cloud_backup-*` snapshots accumulating | The sweep is not running. Check `apply.log` for the nested patch applying, and confirm `sync.py` carries the `TRUECLOUD_PATCH` block. |
+| Web UI blank after a patch | A bad pattern unbalanced the bundle. `apply.sh` now refuses to write in that case, but if you hit it on an older version: restore `chunk-*.js.pre-truecloud-patch` over the live chunk, then re-run `install.sh`. (`MARKER` makes an already-patched file skip, so the patch cannot heal a corrupted bundle by itself.) |
 | Stale mounts under `/run/truecloud-nested` | A crashed run. The next backup tears them down. To clear them now: `python3 patch/truecloud_nested.py cleanup` (also run by `uninstall.sh` and `recover.sh`). It names any ZFS snapshot an interrupted run left pinned. |
 
 ## Supported providers after patching

install.sh

@@ -18,7 +18,7 @@
 
 set -euo pipefail
 
-VERSION="0.3.0"
+VERSION="0.3.1"
 
 # The directory containing install.sh is the permanent install location.
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"

patch/apply.sh

@@ -32,7 +32,7 @@
 # Derive PATCH_DIR from this script's location (parent of the patch/ directory).
 PATCH_DIR="$(cd "$(dirname "$0")/.." && pwd)"
 LOG="$PATCH_DIR/apply.log"
-VERSION="0.3.0"
+VERSION="0.3.1"
 
 # Rotate log at 512 KB to avoid unbounded growth on a system volume.
 # Keep two prior generations (.1 and .2) so the last three boots are always available.

patch/patch_ui.py

@@ -9,8 +9,8 @@ in the minified JS in one of two forms depending on TrueNAS / Angular version:
   TrueNAS 24.x (static inline array):
       "filterByProviders",["STORJ_IX"]
 
-  TrueNAS 25.x+ (Angular pureFunction binding):
-      "filterByProviders",pe(115,Rn,i.CloudSyncProviderName.Storj)
+  TrueNAS 25.x+ (Angular pureFunction binding, inside a chained property call):
+      c(2,"filterByProviders",pe(115,Rn,i.CloudSyncProviderName.Storj))("required",!0)
 
 Both are replaced so the dropdown includes S3 and B2. The file is backed up
 before modification so uninstall.sh can restore it.
@@ -33,11 +33,21 @@ WEBUI_CANDIDATES = [
 # Patterns tried in order; the first match wins.
 # Each entry is (compiled_regex, replacement_string).
 _PATTERNS = [
-    # TrueNAS 25.x+: Angular emits a pureFunction call instead of a literal array.
+    # TrueNAS 25.x+: Angular emits a pureFunction call instead of a literal array,
+    # inside a CHAINED property binding — so the call is followed by two closing
+    # parens, one for pe(...) and one for the property(...) it sits in:
+    #
+    #     c(2,"filterByProviders",pe(115,Rn,i.CloudSyncProviderName.Storj))("required",!0)
+    #                                                                    ^^
+    # The pattern consumes both and re-emits one, leaving the paren balance
+    # unchanged. Getting that wrong is a syntax error in the bundle and the whole
+    # web UI goes blank — see tests/test_patch_ui.py.
+    #
+    # The minified names (pe / slot index / Rn / i) change across builds;
+    # CloudSyncProviderName.Storj is stable because it is a TypeScript enum name.
     (re.compile(r'("filterByProviders",)\w+\(\d+,\w+,\w+\.CloudSyncProviderName\.Storj\)\)'),
      r'\1["STORJ_IX","S3","B2"])'),
 
-
     # TrueNAS 24.x and earlier: static inline array.
     (re.compile(r'("filterByProviders",)\["STORJ_IX"\]'),
      r'\1["STORJ_IX","S3","B2"]'),
@@ -55,6 +65,11 @@ def _match_pattern(content):
     return None, None
 
 
+def _paren_delta(s):
+    """Net parenthesis balance. Patching must not change it — see main()."""
+    return s.count("(") - s.count(")")
+
+
 def find_bundle():
     """
     Search WEBUI_CANDIDATES for the JS chunk containing the filterByProviders
@@ -125,6 +140,24 @@ def main():
         )
         return
 
+    # Never write JS whose parentheses we have unbalanced. A pattern that eats one
+    # paren too many is a syntax error in the bundle and the entire TrueNAS web UI
+    # goes blank -- and because MARKER is then present, every later run reports
+    # "already patched" and skips, so the patch cannot heal itself. Recovery means
+    # hand-restoring the .pre-truecloud-patch backup.
+    #
+    # This is not hypothetical: it shipped once. Refuse instead.
+    if _paren_delta(patched) != _paren_delta(content):
+        print(
+            "[truecloud-patch] ERROR: the replacement would unbalance the bundle's "
+            "parentheses — refusing to write.\n"
+            "[truecloud-patch] The UI is UNCHANGED and still works. This means the "
+            "pattern no longer fits this TrueNAS build.\n"
+            "[truecloud-patch] File an issue at "
+            "https://github.com/sudolulo/truenas-truecloud-patch"
+        )
+        return
+
     tmp = path + ".tmp"
     try:
         with open(tmp, "w", encoding="utf-8") as fh:

patch/truecloud_nested.py

@@ -371,6 +371,20 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None):
     """
     dataset = snapshot.partition("@")[0]
 
+    # Fast path: ONE recursive delete removes the parent and every child that
+    # `zfs snapshot -r` created (252 on a real pool). Deleting them individually
+    # also works, but it is neither cheap nor atomic -- a run killed part-way
+    # through 252 sequential deletes leaves exactly the orphans this function
+    # exists to prevent.
+    try:
+        await middleware.call("zfs.snapshot.delete", snapshot, {"recursive": True})
+        return
+    except Exception:  # noqa: BLE001 - fall through to the explicit sweep
+        pass
+
+    # The parent may already be gone -- stock's `finally` can win the race once
+    # our mounts are released -- which fails the recursive delete while the
+    # children survive. Sweep them by name.
     try:
         snaps = await middleware.call(
             "zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}

recover.sh

@@ -17,7 +17,7 @@
 #   bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh
 #   systemctl restart middlewared
 
-VERSION="0.3.0"
+VERSION="0.3.1"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 

tests/test_patch_ui.py

@@ -0,0 +1,171 @@
+"""Tests for the Angular bundle patch.
+
+This is the one part of the patch that edits *minified third-party JavaScript* by
+regex, so it is the easiest place to silently produce a broken bundle: a pattern
+that matches nothing leaves the dropdown Storj-only, and a pattern that matches
+sloppily can unbalance the parentheses and take the whole web UI down.
+
+Nothing checked it until now. The snippets below are verbatim from a real
+TrueNAS 25.x bundle (chunk-*.js, pre-patch).
+"""
+
+import os
+import re
+import sys
+
+import pytest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "patch"))
+
+from patch_ui import MARKER, _PATTERNS, _match_pattern  # noqa: E402
+
+# Verbatim from /usr/share/truenas/webui/chunk-FX2QXNQU.js on TrueNAS 25.10.
+# Angular emits the binding as a chained ɵɵproperty(...)(...) call, so the
+# pureFunction call is followed by TWO closing parens: one for pe(...), one for
+# property(...).
+REAL_25X = (
+    'c(2,"filterByProviders",pe(115,Rn,i.CloudSyncProviderName.Storj))'
+    '("required",!0),r(3'
+)
+
+# TrueNAS 24.x and earlier emitted a literal array.
+REAL_24X = 'c(2,"filterByProviders",["STORJ_IX"])("required",!0),r(3'
+
+
+def apply_patch(content):
+    """Run the same match-and-substitute main() does."""
+    find, replace = _match_pattern(content)
+    assert find is not None, "no pattern matched"
+    patched, count = find.subn(replace, content)
+    return patched, count
+
+
+def paren_delta(s):
+    """Net paren balance. The snippets are fragments of a minified file, so they
+    are not balanced on their own -- what must hold is that patching does not
+    CHANGE the balance. Consuming one paren too many is a syntax error in the
+    bundle, and the whole TrueNAS web UI goes blank."""
+    return s.count("(") - s.count(")")
+
+
+@pytest.mark.parametrize("source", [REAL_25X, REAL_24X], ids=["25.x", "24.x"])
+class TestAgainstRealBundles:
+    def test_matches_exactly_once(self, source):
+        # main() refuses to write unless count == 1 — more than one match would
+        # mean the pattern is too loose to trust against a minified bundle.
+        _patched, count = apply_patch(source)
+        assert count == 1
+
+    def test_result_contains_all_three_providers(self, source):
+        patched, _ = apply_patch(source)
+        assert MARKER in patched
+        assert '"filterByProviders",["STORJ_IX","S3","B2"]' in patched
+
+    def test_patch_does_not_change_paren_balance(self, source):
+        # Consuming one paren too many (or too few) is a syntax error in the
+        # bundle and the entire TrueNAS web UI goes blank. This is the invariant
+        # the 25.x pattern has to get right: it eats `pe(...)` which sits inside
+        # a chained property(...)(...) call.
+        patched, _ = apply_patch(source)
+        assert paren_delta(patched) == paren_delta(source)
+
+    def test_surrounding_code_is_untouched(self, source):
+        patched, _ = apply_patch(source)
+        assert patched.startswith("c(2,")
+        assert patched.endswith('("required",!0),r(3')
+
+    def test_patch_is_idempotent(self, source):
+        # apply.sh re-runs every boot; MARKER short-circuits an already-patched
+        # file, but the pattern must also not match its own output.
+        patched, _ = apply_patch(source)
+        find, _replace = _match_pattern(patched)
+        if find is not None:
+            # Only the 24.x literal-array pattern may still "match" — and only if
+            # it would produce the same text. Anything else means double-patching.
+            again, _ = apply_patch(patched)
+            assert again == patched, "re-patching must be a no-op"
+
+
+def test_storj_only_bundle_is_recognised():
+    assert _match_pattern(REAL_25X)[0] is not None
+
+
+def test_unrelated_javascript_is_never_touched():
+    # A pattern loose enough to hit unrelated code would corrupt the bundle.
+    for noise in (
+        'c(2,"filterByProviders",pe(115,Rn,i.SomethingElse.Storj))',
+        'c(2,"otherBinding",pe(115,Rn,i.CloudSyncProviderName.Storj))',
+        '"filterByProviders"',
+    ):
+        find, _ = _match_pattern(noise)
+        assert find is None, f"pattern must not match: {noise}"
+
+
+def test_every_pattern_is_anchored_to_filterbyproviders():
+    # Guards against a future pattern broad enough to rewrite arbitrary JS.
+    for find, _replace in _PATTERNS:
+        assert "filterByProviders" in find.pattern
+
+
+def test_patterns_compile_and_replacements_reference_group_one():
+    for find, replace in _PATTERNS:
+        assert isinstance(find, re.Pattern)
+        assert r"\1" in replace, "replacement must preserve the binding name"
+
+
+class TestCorruptionGuard:
+    """A bad pattern must never reach the bundle.
+
+    This is not hypothetical. Commit 47cdf72 shipped a pattern that consumed one
+    closing paren and emitted one, netting an extra `)`:
+
+        c(2,"filterByProviders",["STORJ_IX","S3","B2"]))("required",!0)
+                                                      ^^ syntax error
+
+    The web UI went blank. And because MARKER was then present in the file, every
+    subsequent run reported "already patched" and skipped — so the patch could not
+    heal itself, and the bundle had to be hand-restored from the backup.
+    """
+
+    # Verbatim from 47cdf72.
+    BROKEN = (
+        re.compile(r'("filterByProviders",)\w+\(\d+,\w+,\w+\.CloudSyncProviderName\.Storj\)'),
+        r'\1["STORJ_IX","S3","B2"])',
+    )
+
+    def test_the_regression_that_blanked_the_ui_is_detectable(self):
+        find, replace = self.BROKEN
+        patched, count = find.subn(replace, REAL_25X)
+        assert count == 1, "it did match — that is why it got written"
+        assert paren_delta(patched) != paren_delta(REAL_25X), (
+            "the paren balance changes; this is the signal main() now refuses on"
+        )
+
+    def test_main_refuses_to_write_an_unbalanced_bundle(self, monkeypatch, tmp_path, capsys):
+        import patch_ui
+
+        bundle = tmp_path / "chunk-TEST.js"
+        bundle.write_text(REAL_25X, encoding="utf-8")
+
+        monkeypatch.setattr(patch_ui, "WEBUI_CANDIDATES", [str(tmp_path)])
+        monkeypatch.setattr(patch_ui, "_PATTERNS", [self.BROKEN])
+
+        patch_ui.main()
+
+        out = capsys.readouterr().out
+        assert "refusing to write" in out
+        # The bundle must be byte-for-byte untouched — a broken UI is far worse
+        # than an unpatched one.
+        assert bundle.read_text(encoding="utf-8") == REAL_25X
+
+    def test_a_good_pattern_still_writes(self, monkeypatch, tmp_path):
+        import patch_ui
+
+        bundle = tmp_path / "chunk-TEST.js"
+        bundle.write_text(REAL_25X, encoding="utf-8")
+        monkeypatch.setattr(patch_ui, "WEBUI_CANDIDATES", [str(tmp_path)])
+
+        patch_ui.main()
+
+        assert MARKER in bundle.read_text(encoding="utf-8")
+        assert (tmp_path / "chunk-TEST.js.pre-truecloud-patch").exists()

tests/test_release_notes.py

@@ -0,0 +1,122 @@
+"""Tests for the release automation.
+
+The release workflow refuses to publish unless these hold, so a bad tag fails
+loudly in CI instead of shipping a release whose notes are empty, wrong, or whose
+scripts announce a different version than the tag.
+
+That last one is not hypothetical: VERSION= drifted to three different values
+across install.sh / uninstall.sh / recover.sh / apply.sh and nothing noticed.
+"""
+
+import os
+import sys
+
+import pytest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "tools"))
+
+from release_notes import (  # noqa: E402
+    changelog_versions,
+    check,
+    extract_notes,
+    normalise,
+    script_versions,
+)
+
+REPO = os.path.join(os.path.dirname(__file__), "..")
+
+SAMPLE = """\
+# Changelog
+
+## v0.3.0 — 2026-07-13
+
+### Added
+
+- the new thing
+
+## v0.2.1 — 2026-07-09
+
+### Fixed
+
+- the old thing
+
+## v0.2.0 — 2026-07-08
+
+- first
+"""
+
+
+class TestExtractNotes:
+    def test_returns_only_that_versions_body(self):
+        body = extract_notes(SAMPLE, "v0.3.0")
+        assert "the new thing" in body
+        assert "the old thing" not in body
+        # The version heading itself is dropped (GitHub renders its own title),
+        # but sub-headings like "### Added" must survive.
+        assert not body.startswith("## v")
+        assert body.startswith("### Added")
+
+    def test_stops_at_the_next_version_heading(self):
+        body = extract_notes(SAMPLE, "v0.2.1")
+        assert "the old thing" in body
+        assert "first" not in body
+
+    def test_last_section_runs_to_end_of_file(self):
+        assert "first" in extract_notes(SAMPLE, "v0.2.0")
+
+    def test_accepts_the_tag_with_or_without_the_v(self):
+        assert extract_notes(SAMPLE, "0.3.0") == extract_notes(SAMPLE, "v0.3.0")
+
+    def test_unknown_version_raises_rather_than_returning_empty(self):
+        # An empty release body is worse than a failed release.
+        with pytest.raises(KeyError, match="no section"):
+            extract_notes(SAMPLE, "v9.9.9")
+
+
+class TestChangelogVersions:
+    def test_lists_versions_newest_first(self):
+        assert changelog_versions(SAMPLE) == ["0.3.0", "0.2.1", "0.2.0"]
+
+
+class TestAgainstTheRealRepo:
+    """These run against the actual files, so drift breaks the build."""
+
+    def test_every_script_declares_a_version(self):
+        from release_notes import VERSIONED_FILES
+
+        found = script_versions(REPO)
+        missing = [f for f in VERSIONED_FILES if f not in found]
+        assert not missing, f"no VERSION= in: {missing}"
+
+    def test_all_scripts_agree_on_the_version(self):
+        versions = {normalise(v) for v in script_versions(REPO).values()}
+        assert len(versions) == 1, f"scripts disagree on version: {sorted(versions)}"
+
+    def test_the_current_version_has_a_changelog_section(self):
+        version = next(iter({normalise(v) for v in script_versions(REPO).values()}))
+        with open(os.path.join(REPO, "CHANGELOG.md"), encoding="utf-8") as fh:
+            body = extract_notes(fh.read(), version)
+        assert body, f"CHANGELOG.md has no content for v{version}"
+
+    def test_the_current_version_is_the_newest_changelog_entry(self):
+        version = next(iter({normalise(v) for v in script_versions(REPO).values()}))
+        with open(os.path.join(REPO, "CHANGELOG.md"), encoding="utf-8") as fh:
+            newest = changelog_versions(fh.read())[0]
+        assert newest == version, (
+            f"scripts say v{version} but the newest CHANGELOG entry is v{newest}"
+        )
+
+    def test_check_passes_for_the_current_version(self):
+        version = next(iter({normalise(v) for v in script_versions(REPO).values()}))
+        assert check(version, REPO) == []
+
+
+class TestCheckCatchesMistakes:
+    def test_reports_a_tag_that_no_script_matches(self):
+        problems = check("v9.9.9", REPO)
+        assert problems
+        assert any("declares VERSION" in p for p in problems)
+
+    def test_reports_a_missing_changelog_section(self):
+        problems = check("v9.9.9", REPO)
+        assert any("no section" in p for p in problems)

tests/test_truecloud_nested.py

@@ -209,9 +209,16 @@ class FakeMiddleware:
         if method == "zfs.snapshot.query":
             return [{"name": n} for n in self.snapshots]
         if method == "zfs.snapshot.delete":
-            if args[0] not in self.snapshots:
+            name = args[0]
+            opts = args[1] if len(args) > 1 else {}
+            if name not in self.snapshots:
                 raise RuntimeError("does not exist")
-            self.snapshots.remove(args[0])
+            if opts.get("recursive"):
+                # Real `zfs destroy -r` takes the parent and every child snapshot.
+                for n in snapshot_tree_names(name, list(self.snapshots)):
+                    self.snapshots.remove(n)
+            else:
+                self.snapshots.remove(name)
             return True
         raise AssertionError(f"unexpected call {method}")
 
@@ -233,24 +240,35 @@ class TestDeleteSnapshotTree:
         asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
         assert mw.snapshots == []
 
-    def test_survives_query_failure_by_deleting_at_least_the_parent(self):
+    def test_uses_a_single_recursive_delete_not_252_individual_ones(self):
+        # 252 sequential deletes are slow AND not atomic: a run killed part-way
+        # through leaves exactly the orphans this function exists to prevent.
+        mw = FakeMiddleware(["Tap@snap", "Tap/apps@snap", "Tap/apps/lidarr@snap"])
+        asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
+        assert mw.snapshots == []
+        deletes = [a for m, a in mw.calls if m == "zfs.snapshot.delete"]
+        assert len(deletes) == 1, "should be ONE recursive call, not one per snapshot"
+        assert deletes[0][1] == {"recursive": True}
+        assert not [m for m, _a in mw.calls if m == "zfs.snapshot.query"], (
+            "no enumeration needed on the fast path"
+        )
+
+    def test_survives_recursive_and_query_failure_by_deleting_the_parent(self):
         class Broken(FakeMiddleware):
             async def call(self, method, *args):
                 if method == "zfs.snapshot.query":
                     raise RuntimeError("boom")
+                if method == "zfs.snapshot.delete" and len(args) > 1:
+                    raise RuntimeError("recursive delete unavailable")
                 return await super().call(method, *args)
 
         mw = Broken(["Tap@snap"])
         asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
         assert mw.snapshots == []
 
-    def test_attempts_no_delete_when_the_tree_is_already_gone(self):
-        # A successful query returning nothing means there is nothing to do.
-        # Falling back to the parent here would log a spurious "does not exist"
-        # warning on every clean run.
+    def test_leaves_unrelated_snapshots_alone_when_the_tree_is_gone(self):
         mw = FakeMiddleware(["Tap@unrelated"])
         asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
-        assert [m for m, _a in mw.calls if m == "zfs.snapshot.delete"] == []
         assert mw.snapshots == ["Tap@unrelated"]
 
 

tools/release_notes.py

@@ -0,0 +1,157 @@
+#!/usr/bin/env python3
+"""Extract one version's section from CHANGELOG.md, and check version consistency.
+
+Used by .github/workflows/release.yml so a release's body is always the changelog
+entry -- there is no second place to write release notes, and therefore no second
+place for them to be wrong.
+
+    python3 tools/release_notes.py notes v0.3.0     # -> the section body
+    python3 tools/release_notes.py version          # -> version per the scripts
+    python3 tools/release_notes.py check v0.3.0     # -> exit 1 on any mismatch
+"""
+
+from __future__ import annotations
+
+import os
+import re
+import sys
+
+ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+CHANGELOG = os.path.join(ROOT, "CHANGELOG.md")
+
+# Every script prints a version; they must all agree, and agree with the tag.
+# They drifted to three different values once (0.0.4 / 0.2.1) before anything
+# checked them.
+VERSIONED_FILES = [
+    "install.sh",
+    "uninstall.sh",
+    "recover.sh",
+    os.path.join("patch", "apply.sh"),
+]
+
+_VERSION_RE = re.compile(r'^VERSION="([^"]+)"', re.M)
+_HEADING_RE = re.compile(r"^##\s+v?(\d+\.\d+\.\d+[^\s]*)", re.M)
+
+
+def normalise(v: str) -> str:
+    return v.strip().lstrip("v")
+
+
+def script_versions(root: str = ROOT) -> dict[str, str]:
+    """VERSION= as declared by each script."""
+    found = {}
+    for rel in VERSIONED_FILES:
+        path = os.path.join(root, rel)
+        try:
+            with open(path, encoding="utf-8") as fh:
+                m = _VERSION_RE.search(fh.read())
+        except OSError:
+            continue
+        if m:
+            found[rel] = m.group(1)
+    return found
+
+
+def changelog_versions(text: str) -> list[str]:
+    """Versions with a section in the changelog, newest first."""
+    return [normalise(v) for v in _HEADING_RE.findall(text)]
+
+
+def extract_notes(text: str, version: str) -> str:
+    """The body of one version's section, without its heading.
+
+    Raises KeyError if the version has no section -- a release with an empty or
+    wrong body is worse than a failed release.
+    """
+    want = normalise(version)
+    lines = text.splitlines()
+
+    start = None
+    for i, line in enumerate(lines):
+        m = _HEADING_RE.match(line)
+        if m and normalise(m.group(1)) == want:
+            start = i + 1
+            break
+    if start is None:
+        raise KeyError(f"CHANGELOG.md has no section for v{want}")
+
+    end = len(lines)
+    for i in range(start, len(lines)):
+        if _HEADING_RE.match(lines[i]):
+            end = i
+            break
+
+    return "\n".join(lines[start:end]).strip()
+
+
+def check(version: str, root: str = ROOT) -> list[str]:
+    """Every reason this version is not releasable. Empty list means it is."""
+    want = normalise(version)
+    problems = []
+
+    versions = script_versions(root)
+    for rel, got in sorted(versions.items()):
+        if normalise(got) != want:
+            problems.append(f"{rel} declares VERSION={got!r}, tag is v{want}")
+    missing = [r for r in VERSIONED_FILES if r not in versions]
+    for rel in missing:
+        problems.append(f"{rel} has no VERSION= line")
+
+    try:
+        with open(os.path.join(root, "CHANGELOG.md"), encoding="utf-8") as fh:
+            text = fh.read()
+    except OSError as e:
+        problems.append(f"cannot read CHANGELOG.md: {e}")
+        return problems
+
+    try:
+        body = extract_notes(text, want)
+    except KeyError as e:
+        problems.append(str(e))
+    else:
+        if not body:
+            problems.append(f"CHANGELOG.md section for v{want} is empty")
+
+    return problems
+
+
+def main(argv):
+    if len(argv) < 2:
+        print(__doc__, file=sys.stderr)
+        return 2
+
+    cmd = argv[1]
+
+    if cmd == "version":
+        versions = set(map(normalise, script_versions().values()))
+        if len(versions) != 1:
+            print(f"scripts disagree on version: {sorted(versions)}", file=sys.stderr)
+            return 1
+        print(versions.pop())
+        return 0
+
+    if len(argv) < 3:
+        print(f"usage: {argv[0]} {cmd} <version>", file=sys.stderr)
+        return 2
+    version = argv[2]
+
+    if cmd == "notes":
+        with open(CHANGELOG, encoding="utf-8") as fh:
+            print(extract_notes(fh.read(), version))
+        return 0
+
+    if cmd == "check":
+        problems = check(version)
+        for p in problems:
+            print(f"::error::{p}")
+        if problems:
+            return 1
+        print(f"v{normalise(version)} is consistent across scripts and CHANGELOG")
+        return 0
+
+    print(f"unknown command: {cmd}", file=sys.stderr)
+    return 2
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))

uninstall.sh

@@ -3,7 +3,7 @@
 
 set -euo pipefail
 
-VERSION="0.3.0"
+VERSION="0.3.1"
 
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'