CHANGELOG: set v0.3.0 release date

Commit 47cdf72 shipped a pattern that matched one closing paren and emitted one,
netting an extra `)` in the Angular bundle:

    c(2,"filterByProviders",["STORJ_IX","S3","B2"]))("required",!0)
                                                  ^^ syntax error

The TrueNAS web UI went blank. Worse, MARKER was now present in the file, so
every later run reported "already patched" and skipped -- the patch could not
heal itself, and the bundle had to be hand-restored from the .pre-truecloud-patch
backup.

patch_ui.py now compares the parenthesis balance before and after substitution and
refuses to write if it changed. A bundle we cannot patch correctly is left exactly
as it was: an unpatched UI is a missing dropdown entry, a corrupted one is a dead
web UI.

Tests cover the real regression (verbatim 47cdf72 pattern) end to end: it still
matches, the balance still shifts, main() refuses, and the file on disk is
byte-for-byte unchanged. README documents the manual recovery for anyone who
already hit it.

93 tests, ruff and shellcheck clean.

CHANGELOG.md

@@ -1,6 +1,6 @@
 # Changelog
 
-## v0.3.0 — 2026-07-12
+## v0.3.0 — 2026-07-13
 
 ### Added
 
@@ -165,6 +165,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 +187,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

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"]}

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_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"]