The matrix bot would never have worked: wrong permissions, wrong forge

Two bugs, both of which would have failed silently on the first scheduled run: - permissions were while the step pushes a branch and opens a PR. It would have died with a 403 and I would have had a bot that never worked. - it opened the PR on GITHUB, which is a one-way MIRROR. A PR merged there would be clobbered by the next fleet-repos mirror push from Gitea. A bot opening PRs against a mirror is a bot doing nothing, slowly. Now: contents+pull-requests write, and the PR is opened on Gitea (canonical) via its API. One long-lived PR, force-pushed in place -- a daily PR is the same mistake as a daily comment, wearing a hat.
The bug-report bot was spamming; make it say something only when there is something to say
2026-07-13 20:28:30 +00:00 · 2026-07-13 20:20:35 +00:00 · 2026-07-13 19:59:38 +00:00 · 2026-07-13 19:59:35 +00:00 · 2026-07-13 19:56:32 +00:00 · 2026-07-13 19:35:44 +00:00
21 changed files with 1423 additions and 131 deletions
@@ -27,7 +27,10 @@ on:
      - ".github/workflows/compat.yml"

 permissions:
-  contents: read
+  # write, because the matrix refresh pushes a branch and opens a PR. It does NOT get
+  # to move `main` -- that is the whole reason it is a PR. See the refresh step below.
+  contents: write
+  pull-requests: write
  issues: write

 jobs:
@@ -130,17 +133,27 @@ jobs:
      - name: matrix
        run: cat /tmp/matrix.md

-      # Keep the README's table true. A support matrix that quietly goes stale is not
-      # a stale doc -- it is a false promise to somebody deciding whether to trust
-      # this with their backups.
+      # Keep the README's table true — as a PULL REQUEST, on the CANONICAL forge.
      #
-      # Only ever touches the block between the COMPAT MATRIX markers, and only on
-      # the canonical host (Gitea) so the two forges cannot race each other. The
-      # `paths:` trigger above does not include README.md, so this cannot re-trigger
-      # itself; and a README change is documentation-only, which by design raises no
-      # update alert on anyone's box.
-      - name: refresh the README matrix
+      # Two things this gets right that the obvious version gets wrong:
+      #
+      #  1. It is a PR, not a push to main. This used to `git push origin HEAD:main`
+      #     from CI. An unattended write to main is exactly what the release barrier
+      #     exists to prevent — a bot that can move main can move it somewhere nobody
+      #     looked. Nothing lands by itself.
+      #
+      #  2. It runs on GITEA, not GitHub. GitHub is a one-way MIRROR: a PR merged there
+      #     would be silently clobbered by the next `fleet-repos mirror` push from Gitea.
+      #     A bot opening PRs against a mirror is a bot doing nothing, slowly.
+      #
+      # A stale support matrix is not a stale doc — it is a false promise to somebody
+      # deciding whether to trust this with their backups. So it is refreshed daily; it
+      # just asks first.
+      - name: refresh the README matrix (PR on the canonical forge)
        if: ${{ github.event_name == 'schedule' && !contains(github.server_url, 'github.com') }}
+        env:
+          TOKEN: ${{ secrets.GITEA_TOKEN || github.token }}
+          API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
        run: |
          python3 - <<'PY'
          import json, sys
@@ -151,78 +164,73 @@ jobs:
          print("changed" if compat.update_readme(rows) else "unchanged")
          PY

-          if ! git diff --quiet -- README.md; then
+          git diff --quiet -- README.md && { echo "matrix unchanged — nothing to propose"; exit 0; }
+
          git config user.name  "truecloud-patch bot"
          git config user.email "bot@onetick.ninja"
+
+          BRANCH=bot/compat-matrix
+          git checkout -B "$BRANCH"
          git add README.md
          git commit -m "docs: refresh the TrueNAS compatibility matrix"
-            git push origin HEAD:main
-          fi
+          git push -f origin "$BRANCH"

-      # A broken SHIPPED release is an outage: users are on it right now.
+          # ONE long-lived PR, force-pushed in place — not a new one every morning.
+          # (A daily PR is the same mistake as a daily comment, wearing a hat.)
+          BRANCH="$BRANCH" python3 - <<'PY'
+          import json, os, urllib.error, urllib.request
+
+          api, token, branch = os.environ["API"], os.environ["TOKEN"], os.environ["BRANCH"]
+          h = {"Authorization": f"token {token}", "Content-Type": "application/json"}
+
+          def call(url, method="GET", data=None):
+              r = urllib.request.Request(
+                  url, method=method, headers=h,
+                  data=json.dumps(data).encode() if data else None)
+              with urllib.request.urlopen(r) as resp:  # noqa: S310
+                  return json.load(resp) if resp.length != 0 else {}
+
+          existing = [
+              p for p in call(f"{api}/pulls?state=open")
+              if p["head"]["ref"] == branch
+          ]
+          if existing:
+              print(f"PR #{existing[0]['number']} already open; the force-push updated it")
+          else:
+              pr = call(f"{api}/pulls", "POST", {
+                  "head": branch, "base": "main",
+                  "title": "docs: refresh the TrueNAS compatibility matrix",
+                  "body": (
+                      "The daily compatibility check found that the support matrix in "
+                      "the README no longer matches iXsystems' actual middleware.\n\n"
+                      "This only touches the block between the `COMPAT MATRIX` markers. "
+                      "It is regenerated by `tools/compat.py --matrix --update-readme` "
+                      "and force-pushed, so it always reflects the latest run."
+                  ),
+              })
+              print(f"opened PR #{pr['number']}")
+          PY
+
+      # ONE bug report, kept in sync. It is edited in place when the findings change and
+      # says NOTHING when they do not.
+      #
+      # The first version commented on every run and left 11 identical 3,000-character
+      # comments on one issue in a single day. A bot that repeats itself daily gets
+      # muted, and then the next real finding is scrolled past — which defeats the whole
+      # reason for building it.
+      #
+      # Runs on whichever forge it lands on; compat_publish.py handles both, so the two
+      # cannot drift.
+      - name: file / update / close the bug report
+        env:
+          TOKEN: ${{ secrets.GITEA_TOKEN || github.token }}
+          API: ${{ contains(github.server_url, 'github.com') && 'https://api.github.com' || format('{0}/api/v1', github.server_url) }}/repos/${{ github.repository }}
+        run: python3 tools/compat_publish.py --api "$API" --token "$TOKEN" --matrix /tmp/matrix.json
+
+      # A broken SHIPPED release is an outage: users are on it right now. Fails LAST, so
+      # the report is filed before the job goes red.
      - name: fail if a shipped release is broken
        if: ${{ steps.check.outputs.shipped_broken != '0' }}
        run: |
          echo "::error::The patch is broken on a SHIPPED TrueNAS release."
          exit 1
-
-      - name: file a bug report (GitHub)
-        if: ${{ steps.report.outputs.broken == '1' && contains(github.server_url, 'github.com') }}
-        env:
-          GH_TOKEN: ${{ github.token }}
-          TITLE: "Incompatible with upcoming TrueNAS: ${{ steps.report.outputs.refs }}"
-        run: |
-          # One issue per set of broken refs, reopened/updated rather than duplicated
-          # daily -- a bot that files the same issue every morning gets muted, and
-          # then it is not a warning system any more.
-          existing="$(gh issue list --state all --search "$TITLE" \
-                        --json number,title \
-                        --jq '.[] | select(.title == env.TITLE) | .number' | head -1)"
-
-          if [ -n "$existing" ]; then
-            gh issue comment "$existing" --body-file /tmp/issue.md
-            gh issue reopen "$existing" 2>/dev/null || true
-          else
-            gh issue create --title "$TITLE" --body-file /tmp/issue.md
-          fi
-
-      - name: file a bug report (Gitea)
-        if: ${{ steps.report.outputs.broken == '1' && !contains(github.server_url, 'github.com') }}
-        env:
-          TOKEN: ${{ secrets.GITEA_TOKEN || github.token }}
-          API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
-          TITLE: "Incompatible with upcoming TrueNAS: ${{ steps.report.outputs.refs }}"
-        run: |
-          # python3, not jq: jq is not guaranteed on a self-hosted runner, and a bug
-          # report that dies on a missing tool is a warning system that does not warn.
-          python3 - <<'PY'
-          import json, os, urllib.error, urllib.request
-
-          api, token, title = os.environ["API"], os.environ["TOKEN"], os.environ["TITLE"]
-          with open("/tmp/issue.md", encoding="utf-8") as fh:
-              body = fh.read()
-          headers = {"Authorization": f"token {token}",
-                     "Content-Type": "application/json"}
-
-          def call(url, method, data=None):
-              req = urllib.request.Request(
-                  url, method=method, headers=headers,
-                  data=json.dumps(data).encode() if data else None)
-              with urllib.request.urlopen(req) as r:  # noqa: S310
-                  return json.load(r) if r.length != 0 else {}
-
-          # Same title => same issue. Comment on it rather than filing a new one every
-          # morning: a bot that duplicates itself daily gets muted, and then it is not
-          # a warning system any more.
-          issues = call(f"{api}/issues?state=all&type=issues", "GET")
-          match = next((i for i in issues if i["title"] == title), None)
-
-          if match:
-              n = match["number"]
-              call(f"{api}/issues/{n}/comments", "POST", {"body": body})
-              call(f"{api}/issues/{n}", "PATCH", {"state": "open"})
-              print(f"commented on and reopened issue #{n}")
-          else:
-              made = call(f"{api}/issues", "POST", {"title": title, "body": body})
-              print(f"filed issue #{made['number']}")
-          PY
@@ -6,6 +6,35 @@ is deliberate: see [Releasing](docs/releasing.md). Twelve releases were cut on
 live, every one of those interrupts every user. An alert people learn to ignore is
 worse than no alert, because one day it carries a security fix.

+## v0.6.1 — 2026-07-13
+### Fixed
+
+- **A reboot mid-backup orphaned the entire snapshot tree, permanently.** The sidecar
+  is the record of which snapshots a run pinned — and it lives in `/run`, which is
+  **tmpfs**. A reboot (or a crash) between taking the recursive snapshot and cleaning
+  it up destroyed that record, leaving one snapshot per descendant dataset — **250+ on
+  a real pool** — with nothing left pointing at them. Nothing would ever have found
+  them again.
+
+  `gc_stale_snapshots()` is the backstop: it identifies leftovers **by name**, so it
+  works when the record is gone. It runs at the start of every backup, after the
+  sidecar reclaim — the recorded path stays authoritative, and the collector only ever
+  mops up what the record lost.
+
+  Because it deletes data on a *name match* — a weaker claim than a recorded fact — the
+  selection is a **pure function** with the harshest tests in the suite. A snapshot is
+  collected only if **all** of these hold:
+
+  | | |
+  | --- | --- |
+  | name is exactly `<dataset>@<task>-<YYYYMMDDHHMMSS>` | so `cloud_backup-5` never matches `cloud_backup-50`, an `auto-*` periodic snapshot, or anything a human made |
+  | it is not the current run's | parent *and* children are excluded |
+  | **nothing is mounted from it** | an in-flight run pins its own snapshots — this, not the age guard, is what protects a concurrent backup |
+  | it is **over an hour old** | covers the seconds-long window where a live run has snapshotted but not yet mounted |
+
+  Verified against the real pool: of **4,728** snapshots — including **2,341** periodic
+  ones — it selects exactly the orphans of the task being run, and nothing else.
+
 ## v0.6.0 — 2026-07-13
 ### Added

@@ -66,6 +95,50 @@ worse than no alert, because one day it carries a security fix.

 ### Fixed

+- **A few snapshots leaked on every nested run, forever.** Found on real hardware, in
+  the one place it could be: a 256-snapshot backup of `/mnt/Tap` swept 253 cleanly and
+  left **3 behind** with `dataset is busy`.
+
+  The cause is ZFS's own automount. Reading anything under
+  `<dataset>/.zfs/snapshot/<snap>/` makes ZFS **automount that snapshot**, and it stays
+  mounted for `zfs_expire_snapshot` seconds (**300** by default) after the last access.
+  `teardown()` unmounts *our* bind mounts — but not the automount underneath — so
+  `zfs destroy` refuses for exactly the datasets restic read most recently. Then
+  `cleanup_task()` removed the sidecar anyway, destroying the only record that those
+  snapshots existed. Nothing would ever have reclaimed them.
+
+  Three changes, and the third is the one that makes it safe rather than merely
+  unlikely:
+  - `release_snapdirs()` unmounts ZFS's own `.zfs/snapshot` automounts (deepest first)
+    before deleting, so the snapshots are not busy in the first place.
+  - `delete_snapshot_tree()` **retries** the transient busy, and **returns the
+    snapshots it could not delete** instead of swallowing them.
+  - **The sidecar is now removed only on a confirmed-clean sweep** — including on the
+    staging-failure path, which used to remove it *before* the caller swept. The
+    asymmetry is deliberate: a sidecar left behind when the tree is already gone costs
+    one no-op delete on the next run, while a sidecar removed while the tree still
+    exists is unrecoverable. Survivors are reclaimed by the next run.
+
+  **Expect the occasional straggler, and expect it to clean itself up.** On a
+  256-snapshot tree this reliably sweeps ~255 immediately and may leave **one**: it is
+  whatever restic read last, so its 300-second window has barely opened. That one is
+  logged, its sidecar is kept, and the next run reclaims it before doing anything else.
+  The leak is bounded at a single cycle rather than growing without limit — which is
+  the property that actually matters. Blocking a backup job for five minutes to chase
+  the last snapshot would be a worse trade, so it is not made.
+
+- **Installing the patch permanently blocked updating it.** `install.sh` does
+  `chmod +x update.sh`, and git recorded `update.sh` as `100644` — so the chmod was a
+  *tracked modification*, and `update.sh` refuses to run over a dirty tree. Install
+  once and you could never update again; the error even told you to run
+  `git checkout -- .`, which just undoes the exec bit so the next install can re-dirty
+  it. A real box sat on an old version for exactly this reason.
+
+  Fixed on both sides: the scripts `install.sh` chmods are now executable in git (so
+  the chmod is a no-op), and `update.sh`'s dirty check now looks at **content**, not
+  file mode — `git diff --numstat` reports `0 0` for a mode-only change. A test
+  asserts every script in `install.sh`'s chmod loop is already `100755` in git.
+
 - **Nested snapshots were broken on TrueNAS 24.10 and 25.04, and had been all
  along.** `SYNC_BLOCK`'s wrapper spelled out the stock signature and forwarded five
  arguments — but those releases declare `restic_backup(middleware, job,
@@ -22,7 +22,7 @@ Clone it onto a **pool** (not the boot device — that is wiped on TrueNAS upgra
 then run `install.sh` as root:

 ```bash
-git clone https://git.onetick.ninja/flan/truenas-truecloud-patch.git \
+git clone https://github.com/sudolulo/truenas-truecloud-patch.git \
    /mnt/tank/truenas-truecloud-patch          # replace `tank` with your pool
 cd /mnt/tank/truenas-truecloud-patch
 sudo bash install.sh
@@ -158,7 +158,7 @@ python3 /mnt/tank/truenas-truecloud-patch/patch/create_task.py verify
 | What you see | What it means |
 |---|---|
 | `[OK] providers`, `[OK]`/`[SKIP] nested_snapshots` | Fine. Nothing to do. |
-| `WARNING: … pattern not found` (UI) | The Angular bundle changed. The UI dropdown reverts to Storj-only, but **backups keep working** — create tasks with `create_task.py` meanwhile, and [open an issue](https://git.onetick.ninja/flan/truenas-truecloud-patch/issues) with your TrueNAS version. |
+| `WARNING: … pattern not found` (UI) | The Angular bundle changed. The UI dropdown reverts to Storj-only, but **backups keep working** — create tasks with `create_task.py` meanwhile, and [open an issue](https://github.com/sudolulo/truenas-truecloud-patch/issues) with your TrueNAS version. |
 | `WARNING: truecloud-patch is NOT COMPATIBLE with this TrueNAS version` | This TrueNAS changed middleware underneath the patch, and the named module was **deliberately not applied** — see `incompatible.json` for exactly which assumption broke. TrueNAS is left stock, so nothing is half-patched. Check [TrueNAS compatibility](../README.md#truenas-compatibility), then `bash update.sh` once a release supports your version; it re-applies itself on the next boot. This is **not** the kill switch and needs no manual reset. |
 | `[FAIL] providers` | **Your B2/S3 backups will not run.** middlewared is fine, but the credential/URL handling is gone. Open an issue with your version. |
 | `[FAIL] nested_snapshots` | The stock guard is back, so tasks with `snapshot = true` on a nested dataset will fail validation. Turn the option off on those tasks until it's fixed. |
@@ -95,6 +95,40 @@ re-scan each time.

 ### Snapshot lifecycle

+> **Two mechanisms clean up, and the second exists because the first can be destroyed.**
+>
+> 1. **The sidecar** records exactly which snapshots a run pinned, and is removed only
+>    on a confirmed-clean sweep. Precise, and it survives a middlewared restart.
+> 2. **The garbage collector** finds leftovers by *name*, so it still works when the
+>    sidecar is gone — and it can be: **the sidecar lives in `/run`, which is tmpfs.** A
+>    reboot mid-backup takes it, and with it the only record of a 250-snapshot tree.
+>
+> The collector runs at the start of every backup, after the sidecar reclaim. It will
+> only touch a snapshot named `<dataset>@<task>-<timestamp>` that is not the current
+> run's, has **nothing mounted from it** (which is what protects a concurrently-running
+> backup), and is **over an hour old**. Periodic `auto-*` snapshots, other tasks'
+> snapshots, and anything you made by hand are structurally out of reach.
+
+
+> **A snapshot may survive a run, and that is expected.** ZFS **automounts**
+> `<dataset>/.zfs/snapshot/<snap>` the moment it is read, and holds it for
+> `zfs_expire_snapshot` seconds (**300** by default) after the last access. So
+> whatever restic read *last* is still pinned when we try to destroy it, and
+> `zfs destroy` refuses with `dataset is busy`.
+>
+> The patch unmounts those automounts itself and retries, which clears ~255 of 256 on
+> a real pool. The one that remains is **logged, its sidecar is kept, and the next run
+> reclaims it before doing anything else** — so the leak is bounded at a single cycle
+> instead of growing forever. Seeing one `could not delete snapshot … it will be
+> reclaimed on the next run` in the log is normal. Seeing the count *grow* run over run
+> is not, and would be a bug.
+>
+> This is why the sidecar is removed **only on a confirmed-clean sweep**: it is the
+> only record those snapshots exist, and a run that dropped it while they were still
+> around would orphan them permanently. That is precisely what happened before this was
+> fixed.
+
+
 `zfs.snapshot.delete` defaults to **`recursive=False`**, and stock
 `restic_backup()` calls it with no options. Stock is safe only because its
 validation means a *recursive* snapshot never actually happens in the field.
@@ -109,7 +109,7 @@ If a module shows `[FAIL]`:
   risk.
 4. **If the detail says the module doesn't exist**, a TrueNAS update renamed
   or restructured the internal API.
-   [Open an issue](https://git.onetick.ninja/flan/truenas-truecloud-patch/issues)
+   [Open an issue](https://github.com/sudolulo/truenas-truecloud-patch/issues)
   with your TrueNAS version number and the full verify output.

 ---
@@ -4,7 +4,7 @@
 # Prerequisites: run as root on TrueNAS SCALE with middlewared running.
 # Clone this repository to a persistent ZFS pool first:
 #
-#   git clone https://git.onetick.ninja/flan/truenas-truecloud-patch \
+#   git clone https://github.com/sudolulo/truenas-truecloud-patch \
 #       /mnt/<pool>/truenas-truecloud-patch
 #   cd /mnt/<pool>/truenas-truecloud-patch && bash install.sh
 #
@@ -18,7 +18,7 @@

 set -euo pipefail

-VERSION="0.6.0"
+VERSION="0.6.1"

 # The directory containing install.sh is the permanent install location.
 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
@@ -72,7 +72,7 @@ done
 if [ ! -f "$PATCH_DIR/patch/apply.sh" ]; then
    echo "ERROR: patch files not found at $PATCH_DIR/patch/" >&2
    echo "Run install.sh from a clone of the repository on a persistent pool:" >&2
-    echo "  git clone https://git.onetick.ninja/flan/truenas-truecloud-patch \\" >&2
+    echo "  git clone https://github.com/sudolulo/truenas-truecloud-patch \\" >&2
    echo "      /mnt/<pool>/truenas-truecloud-patch" >&2
    echo "  cd /mnt/<pool>/truenas-truecloud-patch && bash install.sh" >&2
    exit 1
@@ -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.6.0"
+VERSION="0.6.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.
@@ -52,7 +52,7 @@ import subprocess
 import sys
 import time

-__version__ = "0.6.0"
+__version__ = "0.6.1"

 _PATCH_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 _STATUS_FILE = os.path.join(_PATCH_DIR, "hook_status.json")
@@ -111,7 +111,7 @@ def main():
        print(
            "[truecloud-patch] WARNING: filterByProviders pattern not found in any JS bundle.\n"
            "[truecloud-patch] The TrueNAS webui may have been restructured in this version.\n"
-            "[truecloud-patch] File an issue at https://git.onetick.ninja/flan/truenas-truecloud-patch\n"
+            "[truecloud-patch] File an issue at https://github.com/sudolulo/truenas-truecloud-patch\n"
            f"[truecloud-patch] TrueNAS version info: {_tnversion()}"
        )
        return
@@ -136,7 +136,7 @@ def main():
            f"[truecloud-patch] WARNING: {count} replacement(s) in {path}; "
            f"expected exactly 1 — skipping write to avoid corrupting the bundle.\n"
            f"[truecloud-patch] File an issue at "
-            f"https://git.onetick.ninja/flan/truenas-truecloud-patch"
+            f"https://github.com/sudolulo/truenas-truecloud-patch"
        )
        return

@@ -154,7 +154,7 @@ def main():
            "[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://git.onetick.ninja/flan/truenas-truecloud-patch"
+            "https://github.com/sudolulo/truenas-truecloud-patch"
        )
        return

@@ -55,9 +55,11 @@ Therefore this module owns the whole lifecycle:
 from __future__ import annotations

 import contextlib
+import datetime
 import os
 import stat
 import subprocess
+import time

 __all__ = [
    "STAGING_BASE",
@@ -67,10 +69,13 @@ __all__ = [
    "cleanup_task",
    "current_mounts_under",
    "delete_snapshot_tree",
+    "gc_stale_snapshots",
+    "mounted_snapshots",
    "plan_staging",
    "sidecar_for",
    "snapshot_tree_names",
    "stage_nested",
+    "stale_snapshot_names",
    "staging_root_for",
    "teardown",
    "verify_staged",
@@ -113,21 +118,39 @@ def sidecar_for(staging_root: str) -> str:
    return staging_root + ".snapshot"


-def _write_sidecar(staging_root: str, snapshot: str) -> None:
-    """Record the pinned snapshot on disk. Blocking; call via run_in_thread."""
+def _write_sidecar(staging_root: str, snapshots) -> None:
+    """Record every snapshot tree this task still owns. One per line.
+
+    A LIST, not a single name -- and that is not over-engineering, it is a bug fix.
+
+    The sidecar used to hold one snapshot, so a run that reclaimed an older tree,
+    FAILED to finish reclaiming it, and then recorded its own snapshot would
+    **overwrite the only record of the survivor** -- orphaning it permanently, which is
+    exactly the outcome the sidecar exists to prevent. Observed live: a snapshot
+    survived one run, the next run's reclaim also failed (ZFS's 300s automount window
+    had not elapsed, because the runs were minutes apart), and the record was
+    destroyed anyway.
+
+    Now every still-pending tree is carried forward until it is actually gone.
+    """
+    if isinstance(snapshots, str):
+        snapshots = [snapshots]
    with contextlib.suppress(OSError):
        os.makedirs(os.path.dirname(staging_root), exist_ok=True)
        with open(sidecar_for(staging_root), "w", encoding="utf-8") as fh:
-            fh.write(snapshot)
+            fh.write("\n".join(dict.fromkeys(snapshots)))   # de-duped, order kept


-def _read_sidecar(staging_root: str) -> str | None:
-    """The snapshot a previous run recorded here, if any."""
+def _read_sidecar(staging_root: str):
+    """Every snapshot tree a previous run recorded here. [] if none.
+
+    Tolerates the old single-line format, which is just a one-element list.
+    """
    try:
        with open(sidecar_for(staging_root), encoding="utf-8") as fh:
-            return fh.read().strip() or None
+            return [ln.strip() for ln in fh if ln.strip()]
    except OSError:
-        return None
+        return []


 def _remove_sidecar(staging_root: str) -> None:
@@ -157,6 +180,76 @@ def snapshot_tree_names(snapshot: str, all_names) -> list[str]:
    ]


+#: A snapshot must be at least this old before the garbage collector will touch it.
+#:
+#: The GC identifies our leftovers by NAME, so its only real risk is deleting a
+#: snapshot belonging to a run that is still starting up -- the window between
+#: `zfs snapshot -r` and the bind mounts appearing, which is seconds. An hour is three
+#: orders of magnitude more slack than that window needs, and still reclaims a lost
+#: tree on the very next daily run.
+GC_MIN_AGE_SECONDS = 3600
+
+
+def stale_snapshot_names(task_name, current_snapshot, all_names, now,
+                         in_use=(), min_age=GC_MIN_AGE_SECONDS):
+    """Snapshots THIS task created in an earlier run and never cleaned up.
+
+    Pure, because this is the one function here that DELETES DATA on a name match, and
+    a name match is a weaker claim than a recorded fact. Everything it relies on is an
+    argument, so every way it could be wrong is a test.
+
+    Why a garbage collector exists at all, when there is already a sidecar: **the
+    sidecar lives in /run, which is tmpfs.** A reboot mid-backup destroys it, and with
+    it the only record of a 250-snapshot tree. The sidecar handles the normal case
+    precisely; this handles the case where the record itself is gone.
+
+    A snapshot is ours to collect only if ALL of these hold:
+
+      * its name is exactly ``<dataset>@<task_name>-<YYYYMMDDHHMMSS>`` -- so
+        ``cloud_backup-5`` never matches ``cloud_backup-50``'s snapshots, and never
+        matches a periodic ``auto-2026-…`` or anything a human made;
+      * it is not the snapshot the current run is using;
+      * nothing is mounted from it (`in_use`) -- an in-flight run pins its own
+        snapshots, so this alone protects a concurrent one-time backup;
+      * it is older than `min_age` -- which covers the seconds-long window in which a
+        run has taken its snapshot but not yet mounted it.
+
+    `now` is a timezone-aware datetime; timestamps in the name are UTC (stock builds
+    them with `utc_now()`).
+    """
+    prefix = task_name + "-"
+    stale = []
+
+    for name in all_names:
+        _dataset, _, snapname = name.partition("@")
+        if not snapname or not snapname.startswith(prefix):
+            continue
+        if name == current_snapshot or snapname == _snapname_of(current_snapshot):
+            continue
+        if name in in_use:
+            continue
+
+        stamp = snapname[len(prefix):]
+        try:
+            when = datetime.datetime.strptime(stamp, "%Y%m%d%H%M%S").replace(
+                tzinfo=datetime.UTC
+            )
+        except ValueError:
+            # Not our timestamp format. Something else owns this name; leave it alone.
+            continue
+
+        if (now - when).total_seconds() < min_age:
+            continue
+
+        stale.append(name)
+
+    return stale
+
+
+def _snapname_of(snapshot):
+    return snapshot.partition("@")[2] if snapshot else ""
+
+
 def _probe_snapdir(path):
    """Classify a snapshot directory: ``ok``, ``missing``, or why it is unusable.

@@ -362,6 +455,48 @@ def teardown(staging_root, runner=_run, mounts_file="/proc/self/mounts"):
    return errors


+def snapdir_automounts(snapshot_name, mounts_file="/proc/self/mounts"):
+    """Every ``<dataset>/.zfs/snapshot/<snap>`` ZFS automount for this snapshot."""
+    suffix = "/.zfs/snapshot/" + snapshot_name
+    found = []
+    try:
+        with open(mounts_file, encoding="utf-8") as fh:
+            for line in fh:
+                parts = line.split()
+                if len(parts) > 1:
+                    mp = parts[1].replace("\\040", " ")
+                    if mp.endswith(suffix):
+                        found.append(mp)
+    except OSError:
+        return []
+    return sorted(found, key=_depth, reverse=True)      # deepest first
+
+
+def release_snapdirs(snapshot_name, runner=_run, mounts_file="/proc/self/mounts"):
+    """Unmount ZFS's OWN snapshot automounts, so the snapshots can be destroyed.
+
+    Reading anything under ``<dataset>/.zfs/snapshot/<snap>/`` makes ZFS **automount**
+    that snapshot, and it stays mounted for ``zfs_expire_snapshot`` seconds (300 by
+    default) after the last access. teardown() unmounts OUR bind mounts -- but the
+    automount underneath them survives, and while it exists ``zfs destroy`` refuses
+    with *"dataset is busy"*.
+
+    Proven on a real pool: a 256-snapshot recursive tree swept cleanly except for the
+    three datasets restic had read most recently. Those failed with EBUSY, and because
+    cleanup_task removed the sidecar anyway, they were orphaned **permanently** -- a
+    small leak, but a growing one, and exactly the failure this module exists to
+    prevent.
+
+    Deepest first, so a child's automount is released before its parent's.
+    """
+    errors = []
+    for mp in snapdir_automounts(snapshot_name, mounts_file=mounts_file):
+        res = runner(["umount", mp])
+        if res.returncode != 0:
+            errors.append(f"{mp}: {(res.stderr or '').strip()}")
+    return errors
+
+
 # ── orchestration (middleware is duck-typed; no middlewared import) ───────────
 #
 # These are SYNCHRONOUS and talk to middlewared via `middleware.call_sync`, which
@@ -424,15 +559,31 @@ def get_dataset_recursive(datasets, directory):
    )


-def delete_snapshot_tree(middleware, snapshot, logger=None):
+def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
+                         sleep=time.sleep):
    """Delete the parent snapshot AND every child created by ``zfs snapshot -r``.

+    Returns the snapshots it could NOT delete -- callers must not throw that away.
+
    ``zfs.snapshot.delete`` is non-recursive by default and stock calls it with
    no options, so relying on stock would orphan one snapshot per descendant
    dataset on every run. Idempotent: tolerates the parent already being gone
    (stock's ``finally`` may have won the race once our mounts were released).
+
+    "dataset is busy" is EXPECTED here and is TRANSIENT. ZFS automounts
+    ``<dataset>/.zfs/snapshot/<snap>`` when it is read and keeps it mounted for
+    ``zfs_expire_snapshot`` seconds (300 by default) afterwards. So the datasets restic
+    touched last are still pinned when we try to destroy them. We release the
+    automounts explicitly and then retry -- on a real 256-snapshot tree, exactly three
+    snapshots hit this, and before the fix they were orphaned permanently.
    """
-    dataset = snapshot.partition("@")[0]
+    dataset, _, snapname = snapshot.partition("@")
+
+    # Release ZFS's own automounts first, or `zfs destroy` refuses with EBUSY on
+    # everything restic read in the last few minutes.
+    for err in release_snapdirs(snapname):
+        if logger:
+            logger.debug("truecloud-patch: could not release snapdir %s", err)

    # Fast path: ONE recursive delete removes the parent and every child that
    # `zfs snapshot -r` created (252 on a real pool). Deleting them individually
@@ -441,7 +592,7 @@ def delete_snapshot_tree(middleware, snapshot, logger=None):
    # exists to prevent.
    try:
        middleware.call_sync("zfs.snapshot.delete", snapshot, {"recursive": True})
-        return
+        return []
    except Exception as e:  # noqa: BLE001 - fall through to the explicit sweep
        # Usually just "parent already gone" (stock's finally won the race once our
        # mounts were released), which the sweep below handles. Log it rather than
@@ -472,14 +623,127 @@ def delete_snapshot_tree(middleware, snapshot, logger=None):
            )
        names = [snapshot]

-    for name in names:
+    def confirm_gone(failed):
+        """Drop any name ZFS no longer has, even though its delete raised.
+
+        A delete that raised "does not exist" SUCCEEDED as far as we care, and must
+        not be retried or reported. The query is only a refinement: if it cannot be
+        answered we keep the delete's own verdict, rather than inventing survivors --
+        a false survivor keeps the sidecar forever and is reported as a leak that
+        isn't there.
+        """
+        if not failed:
+            return []
+        try:
+            live = middleware.call_sync(
+                "zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
+            )
+        except Exception:  # noqa: BLE001 - cannot refine; trust the delete's verdict
+            return list(failed)
+        live = {s["name"] for s in live}
+        return [n for n in failed if n in live]
+
+    remaining = list(names)
+    for attempt in range(attempts):
+        failed = []
+        for name in remaining:
            try:
                middleware.call_sync("zfs.snapshot.delete", name)
-        except Exception as e:  # noqa: BLE001 - already gone is fine
+            except Exception:  # noqa: BLE001 - busy, or already gone; sorted out below
+                failed.append(name)
+
+        remaining = confirm_gone(failed)
+        if not remaining:
+            return []
+
+        if attempt < attempts - 1:
+            # EBUSY is the automount expiring. Release again (anything that walks
+            # .zfs can re-automount a snapshot) and give it a moment.
+            release_snapdirs(snapname)
+            sleep(5)
+
+    for name in remaining:
        if logger:
            logger.warning(
-                    "truecloud-patch: could not delete snapshot %s: %r", name, e
+                "truecloud-patch: could not delete snapshot %s after %d attempts "
+                "(still busy?) -- it will be reclaimed on the next run",
+                name, attempts,
            )
+    return remaining
+
+
+def mounted_snapshots(mounts_file="/proc/self/mounts"):
+    """Every ZFS snapshot something is currently mounted from.
+
+    The device field of a snapshot mount IS the snapshot name (`Tap/apps/x@snap`), for
+    both our staging bind mounts and ZFS's own .zfs automounts. So this is a direct,
+    factual answer to "is anything using this snapshot right now" -- which is what
+    protects a concurrently-running backup from the garbage collector, rather than
+    trusting an age heuristic to be generous enough.
+    """
+    live = set()
+    try:
+        with open(mounts_file, encoding="utf-8") as fh:
+            for line in fh:
+                dev = line.split(" ", 1)[0]
+                if "@" in dev:
+                    live.add(dev.replace("\\040", " "))
+    except OSError:
+        return set()
+    return live
+
+
+def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
+                       now=None, mounts_file="/proc/self/mounts"):
+    """Delete snapshots this task left behind in an earlier run. Returns what remains.
+
+    The backstop for when the RECORD is gone, not just the snapshots: the sidecar lives
+    in /run (tmpfs), so a reboot mid-backup takes it with them. Without this, that tree
+    -- one snapshot per descendant dataset, 250+ on a real pool -- is orphaned with
+    nothing left pointing at it.
+
+    Selection is `stale_snapshot_names()`, which is pure and heavily tested, because a
+    name match is a weaker claim than a recorded fact and this deletes data on one.
+    """
+    dataset = current_snapshot.partition("@")[0]
+    now = now or datetime.datetime.now(datetime.UTC)
+
+    try:
+        snaps = middleware.call_sync(
+            "zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
+        )
+    except Exception as e:  # noqa: BLE001 - cannot enumerate; collect nothing
+        if logger:
+            logger.warning(
+                "truecloud-patch: could not enumerate snapshots for GC: %r", e
+            )
+        return []
+
+    stale = stale_snapshot_names(
+        task_name, current_snapshot, [s["name"] for s in snaps], now,
+        in_use=mounted_snapshots(mounts_file),
+    )
+    if not stale:
+        return []
+
+    if logger:
+        logger.warning(
+            "truecloud-patch: %d snapshot(s) from an earlier run of %s were never "
+            "cleaned up (a lost record, e.g. a reboot mid-backup); collecting them",
+            len(stale), task_name,
+        )
+
+    remaining = []
+    for name in stale:
+        try:
+            middleware.call_sync("zfs.snapshot.delete", name)
+        except Exception as e:  # noqa: BLE001 - busy, or gone; either way, next run
+            remaining.append(name)
+            if logger:
+                logger.debug(
+                    "truecloud-patch: could not collect %s: %r", name, e
+                )
+    return remaining


 def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
@@ -508,24 +772,50 @@ def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
    # A previous run may have crashed mid-flight; never build on top of that.
    teardown(staging_root)

-    # ...and if it left a sidecar behind, that snapshot tree is still on disk and
-    # nothing else will ever reclaim it. Sweep it before we overwrite the record,
-    # or a single crashed run orphans 160+ snapshots permanently.
-    stale = _read_sidecar(staging_root)
-    if stale and stale != snapshot:
+    # ...and if it left snapshot trees behind, they are still on disk and nothing else
+    # will ever reclaim them. Sweep them before recording our own, or a single crashed
+    # run orphans 160+ snapshots permanently.
+    #
+    # Anything a reclaim FAILS to delete is carried forward, not dropped. Overwriting
+    # the sidecar with only our own snapshot is what destroyed the record of a survivor
+    # once already: the reclaim ran, hit ZFS's 300-second automount window (the runs
+    # were minutes apart), left one snapshot behind, and then the record of it was
+    # overwritten -- a permanent orphan, created by the very code meant to prevent one.
+    pending = []
+    for stale in _read_sidecar(staging_root):
+        if stale == snapshot:
+            continue
        if logger:
            logger.warning(
                "truecloud-patch: reclaiming snapshot tree from an earlier "
-                "interrupted run: %s", stale,
+                "run: %s", stale,
+            )
+        pending.extend(delete_snapshot_tree(middleware, stale, logger=logger))
+
+    if pending and logger:
+        logger.warning(
+            "truecloud-patch: %d snapshot(s) from an earlier run are still busy; "
+            "carrying them forward to the next run", len(pending),
+        )
+
+    # ...and collect anything from an earlier run that has NO record at all.
+    #
+    # The sidecar above is precise but lives in /run, which is tmpfs -- a reboot
+    # mid-backup destroys it and orphans the whole tree with nothing pointing at it.
+    # This finds those by name and is the only thing that ever will.
+    #
+    # It runs AFTER the sidecar reclaim on purpose: the recorded path is authoritative
+    # and cheap, and the GC should only ever be mopping up what the record lost.
+    pending.extend(
+        gc_stale_snapshots(middleware, task_name, snapshot, logger=logger)
    )
-        delete_snapshot_tree(middleware, stale, logger=logger)

    # Record the snapshot BEFORE mounting anything, not after. middlewared can
    # die at any point (this patch even schedules a restart at boot), and the
    # sidecar is the only thing that survives it -- an in-process dict would take
    # the sole record of a 160-snapshot tree with it. Writing it after apply_plan
    # would leave exactly the crash window the sidecar exists to close.
-    _write_sidecar(staging_root, snapshot)
+    _write_sidecar(staging_root, [*pending, snapshot])

    try:
        mounts, skipped = plan_staging(
@@ -541,8 +831,18 @@ def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
        apply_plan(mounts)
        verify_staged(mounts)
    except Exception:
+        # Tear down the mounts, but KEEP the sidecar.
+        #
+        # The caller (SNAPSHOT_BLOCK) sweeps the snapshot tree on the way out, and if
+        # any of it is still busy it will survive -- and the sidecar is the only record
+        # that it exists. Removing it here would orphan those snapshots permanently.
+        #
+        # The asymmetry is deliberate: a sidecar left behind when the tree is already
+        # gone is harmless (the next run tries to delete a tree that is not there,
+        # finds nothing, and moves on), while a sidecar removed while the tree still
+        # exists is unrecoverable. Only a confirmed-clean sweep removes it -- see
+        # cleanup_task().
        teardown(staging_root)
-        _remove_sidecar(staging_root)
        raise

    if logger:
@@ -559,9 +859,9 @@ def cleanup_task(middleware, task_name, logger=None):
    Safe to call unconditionally: a no-op when the task was never staged.
    """
    staging_root = staging_root_for(task_name)
-    snapshot = _read_sidecar(staging_root)
+    pinned = _read_sidecar(staging_root)

-    if snapshot is None and not os.path.isdir(staging_root):
+    if not pinned and not os.path.isdir(staging_root):
        return  # never staged; nothing to do

    errors = teardown(staging_root)
@@ -569,8 +869,39 @@ def cleanup_task(middleware, task_name, logger=None):
        for err in errors:
            logger.warning("truecloud-patch: staging teardown: %s", err)

-    if snapshot is not None:
-        delete_snapshot_tree(middleware, snapshot, logger=logger)
+    if not pinned:
+        _remove_sidecar(staging_root)
+        return
+
+    # Every tree this task still owns -- ours, plus anything an earlier run could not
+    # finish reclaiming.
+    survivors = []
+    for snapshot in pinned:
+        survivors.extend(delete_snapshot_tree(middleware, snapshot, logger=logger))
+
+    # KEEP the sidecar if anything survived. It is the only record that those
+    # snapshots exist, and removing it orphans them permanently.
+    #
+    # That is not theoretical: on a real 256-snapshot tree, three snapshots were still
+    # pinned by ZFS's own .zfs/snapshot automount (which lingers for 300s after the
+    # last read), failed to delete with "dataset is busy", and the sidecar was removed
+    # anyway -- so nothing would ever have reclaimed them. A small leak, but one that
+    # grows by a few snapshots on every single run, forever.
+    #
+    # Left in place, the next run's stage_nested() sees a stale sidecar naming a
+    # different snapshot and sweeps that tree first -- by which time the automounts are
+    # long gone and the delete succeeds.
+    if survivors:
+        if logger:
+            logger.warning(
+                "truecloud-patch: %d snapshot(s) could not be deleted (still busy); "
+                "recording them so the next run reclaims them: %s",
+                len(survivors), ", ".join(survivors),
+            )
+        # The SURVIVORS, not the trees we asked to delete. Writing the original list
+        # back would keep re-sweeping trees that are already gone.
+        _write_sidecar(staging_root, survivors)
+        return

    _remove_sidecar(staging_root)

@@ -599,8 +930,7 @@ def cleanup_all(base=None, runner=_run, mounts_file="/proc/self/mounts",
    # a sidecar is the only record that an interrupted run's snapshot tree (one
    # snapshot per descendant dataset) is still on disk.
    for sc in sorted(glob_fn(os.path.join(base, "*.snapshot"))):
-        snap = read_sidecar(sc[: -len(".snapshot")])
-        if snap:
+        for snap in read_sidecar(sc[: -len(".snapshot")]):
            lines.append(f"  NOTE: an interrupted backup left snapshot '{snap}' behind.")
            lines.append(f"        Remove it and its children:  zfs destroy -r '{snap}'")

@@ -17,7 +17,7 @@
 #   bash /mnt/tank/truenas-truecloud-patch/patch/apply.sh
 #   systemctl restart middlewared

-VERSION="0.6.0"
+VERSION="0.6.1"

 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"

@@ -94,3 +94,40 @@ class TestTheReadmeStaysAReadme:
        assert "24.10" in text[:text.index("## Install")], (
            "the minimum TrueNAS version must be visible above the install steps"
        )
+
+
+class TestInstallDoesNotDirtyTheCheckout:
+    """install.sh chmod +x's scripts. If git records them as 100644, that chmod is a
+    TRACKED MODIFICATION -- and update.sh refuses to run over a dirty tree.
+
+    So installing once permanently blocked updating, for every user, with a message
+    telling them to `git checkout -- .` (which would just undo the exec bit and let
+    the next install re-dirty it). Found on a real box that had been stuck on an old
+    version for exactly this reason.
+
+    Every script install.sh makes executable must already be executable in git.
+    """
+
+    def test_every_chmodded_script_is_already_executable_in_git(self):
+        import re
+        import subprocess
+
+        with open(os.path.join(ROOT, "install.sh"), encoding="utf-8") as fh:
+            m = re.search(r"^for _exe in (.+?); do", fh.read(), re.M)
+        assert m, "could not find install.sh's chmod loop"
+        scripts = m.group(1).split()
+
+        out = subprocess.run(
+            ["git", "ls-files", "-s", *scripts],
+            cwd=ROOT, capture_output=True, text=True, check=True,
+        ).stdout
+
+        not_exec = [
+            line.split("\t")[-1] for line in out.strip().splitlines()
+            if not line.startswith("100755")
+        ]
+        assert not not_exec, (
+            "install.sh chmod +x's these, but git records them as non-executable — "
+            "so installing dirties the checkout and update.sh then refuses to run:\n  "
+            + "\n  ".join(not_exec)
+        )
@@ -9,6 +9,7 @@ across install.sh / uninstall.sh / recover.sh / apply.sh and nothing noticed.
 """

 import os
+import re
 import sys

 import pytest
@@ -233,3 +234,60 @@ class TestCandidateNotesResolveToTheBaseVersion:
    def test_a_genuinely_missing_section_still_raises(self):
        with pytest.raises(KeyError):
            extract_notes(self.CHANGELOG, "v9.9.9-rc1")
+
+
+class TestTheChangelogIsStructurallySound:
+    """The release body IS this file, so a mangled section ships to every user.
+
+    It has been mangled once: an edit matched the literal `## Unreleased` inside a
+    backticked phrase in a prose bullet and spliced a whole new section into the middle
+    of it, splitting the sentence in half.
+    """
+
+    def changelog(self):
+        with open(os.path.join(REPO, "CHANGELOG.md"), encoding="utf-8") as fh:
+            return fh.read()
+
+    def test_no_version_section_is_empty(self):
+        text = self.changelog()
+        for v in changelog_versions(text):
+            assert extract_notes(text, v).strip(), f"v{v} has an empty section"
+
+    def test_versions_are_in_descending_order(self):
+        from release_notes import version_tuple
+        versions = changelog_versions(self.changelog())
+        assert versions == sorted(versions, key=version_tuple, reverse=True), (
+            "CHANGELOG versions are out of order — a section was spliced in wrong"
+        )
+
+    def test_headings_are_at_the_start_of_a_line_and_not_inside_prose(self):
+        # A `### Fixed` that ends up indented under a bullet is a section nobody sees.
+        for i, line in enumerate(self.changelog().splitlines(), 1):
+            if line.lstrip().startswith(("## ", "### ")) and line != line.lstrip():
+                raise AssertionError(
+                    f"line {i}: heading is indented, so it is inside a list item "
+                    f"rather than being a section: {line!r}"
+                )
+
+    def test_every_bullet_that_opens_a_bold_phrase_closes_it(self):
+        # The splice cut `- **A stable release ... under \`## Unreleased` in half,
+        # leaving an unterminated ** and a dangling sentence.
+        #
+        # A bullet is the `- ` line plus everything up to the next top-level bullet or
+        # heading -- bold phrases routinely wrap across lines, so a per-line check
+        # would flag every long bullet in the file.
+        text = self.changelog()
+        bullets = re.split(r"^(?=- |#{2,3} )", text, flags=re.M)
+        bad = []
+        for b in bullets:
+            if not b.startswith("- "):
+                continue
+            # Code spans are not markup: `*args, **kwargs` is a literal, not a bold
+            # phrase, and counting its ** would flag a perfectly well-formed bullet.
+            prose = re.sub(r"`[^`]*`", "", b)
+            if prose.count("**") % 2:
+                bad.append(b.splitlines()[0][:70])
+        assert not bad, (
+            "unbalanced ** in a bullet — a section was probably spliced into the "
+            "middle of it:\n  " + "\n  ".join(bad)
+        )
@@ -348,7 +348,14 @@ class TestStageNestedOrdering:

        assert mw.snapshots == [], "the crashed run's snapshot tree must be reclaimed"

-    def test_sidecar_is_removed_when_staging_fails(self, tmp_path, monkeypatch):
+    def test_sidecar_is_KEPT_when_staging_fails(self, tmp_path, monkeypatch):
+        # The caller sweeps the snapshot tree on the way out, and anything still busy
+        # SURVIVES that sweep -- with the sidecar as its only record. Removing the
+        # sidecar here would orphan those snapshots permanently.
+        #
+        # The asymmetry is the point: a sidecar left behind when the tree is already
+        # gone costs one no-op delete on the next run; a sidecar removed while the tree
+        # still exists is unrecoverable.
        import truecloud_nested as tn

        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
@@ -361,7 +368,12 @@ class TestStageNestedOrdering:
                "cloud_backup-5", DATASETS,
            )

-        assert not os.path.exists(sidecar_for(root))
+        assert os.path.exists(sidecar_for(root)), (
+            "sidecar removed on staging failure — any snapshot the caller's sweep "
+            "cannot delete is now orphaned forever"
+        )
+        with open(sidecar_for(root), encoding="utf-8") as fh:
+            assert fh.read().strip() == "Tap@snap"


 class TestCleanupTask:
@@ -609,3 +621,431 @@ class TestStagingRootFor:
        # os.path.join(BASE, "..") normalises to /run — teardown would rmdir it.
        root = staging_root_for(name)
        assert os.path.normpath(root).startswith("/run/truecloud-nested/")
+
+
+class TestZfsAutomountKeepsSnapshotsBusy:
+    """"dataset is busy" is EXPECTED, TRANSIENT, and used to orphan snapshots forever.
+
+    Reading `<dataset>/.zfs/snapshot/<snap>/` makes ZFS **automount** that snapshot,
+    and it stays mounted for zfs_expire_snapshot seconds (300 by default) after the
+    last access. teardown() unmounts OUR bind mounts, but not the automount underneath
+    -- so `zfs destroy` refuses with EBUSY for everything restic read recently.
+
+    Observed on a real 256-snapshot tree: 253 swept cleanly, and the 3 datasets restic
+    had touched last failed with "dataset is busy". cleanup_task then removed the
+    sidecar anyway, so nothing would ever reclaim them. A few snapshots leaked per run,
+    forever.
+    """
+
+    MOUNTS = (
+        "tmpfs /run tmpfs rw 0 0\n"
+        "Tap/apps/prometheus /mnt/Tap/apps/prometheus/.zfs/snapshot/snap1 zfs ro 0 0\n"
+        "Tap/apps/standing/data /mnt/Tap/apps/standing/data/.zfs/snapshot/snap1 zfs ro 0 0\n"
+        "Tap /mnt/Tap/.zfs/snapshot/snap1 zfs ro 0 0\n"
+        "Tap/other /mnt/Tap/other/.zfs/snapshot/OTHER zfs ro 0 0\n"
+    )
+
+    def _mounts_file(self, tmp_path):
+        p = tmp_path / "mounts"
+        p.write_text(self.MOUNTS)
+        return str(p)
+
+    def test_it_finds_the_automounts_for_this_snapshot_only(self, tmp_path):
+        import truecloud_nested as tn
+        found = tn.snapdir_automounts("snap1", mounts_file=self._mounts_file(tmp_path))
+        assert "/mnt/Tap/other/.zfs/snapshot/OTHER" not in found
+        assert len(found) == 3
+
+    def test_deepest_first(self, tmp_path):
+        # A child's automount must be released before its parent's.
+        import truecloud_nested as tn
+        found = tn.snapdir_automounts("snap1", mounts_file=self._mounts_file(tmp_path))
+        assert found[-1] == "/mnt/Tap/.zfs/snapshot/snap1"
+
+    def test_release_snapdirs_unmounts_them(self, tmp_path):
+        import truecloud_nested as tn
+        called = []
+
+        class R:
+            returncode = 0
+            stderr = ""
+
+        def runner(cmd):
+            called.append(cmd)
+            return R()
+
+        errs = tn.release_snapdirs("snap1", runner=runner,
+                                   mounts_file=self._mounts_file(tmp_path))
+        assert errs == []
+        assert all(c[0] == "umount" for c in called)
+        assert len(called) == 3
+
+
+class BusyMiddleware(FakeMiddleware):
+    """Deletes fail with EBUSY until `busy_until_attempt` passes -- like a ZFS
+    automount expiring."""
+
+    def __init__(self, snapshots, busy, busy_for=2):
+        super().__init__(snapshots)
+        self.busy = set(busy)
+        self.busy_for = busy_for
+        self.attempts = 0
+
+    def call_sync(self, method, *args):
+        if method == "zfs.snapshot.delete":
+            name = args[0]
+            opts = args[1] if len(args) > 1 else {}
+            if opts.get("recursive"):
+                raise RuntimeError("cannot destroy snapshot: dataset is busy")
+            if name in self.busy:
+                self.attempts += 1
+                if self.attempts <= self.busy_for * len(self.busy):
+                    raise RuntimeError(f"cannot destroy '{name}': dataset is busy")
+        return super().call_sync(method, *args)
+
+
+class TestDeleteRetriesAndReportsSurvivors:
+    def test_a_transient_busy_is_retried_and_wins(self, monkeypatch):
+        import truecloud_nested as tn
+        monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
+
+        mw = BusyMiddleware(
+            ["Tap@snap", "Tap/apps@snap", "Tap/apps/prometheus@snap"],
+            busy=["Tap/apps/prometheus@snap"], busy_for=1,
+        )
+        survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
+        assert survivors == []
+        assert mw.snapshots == []
+
+    def test_a_permanently_busy_snapshot_is_REPORTED_not_swallowed(self, monkeypatch):
+        import truecloud_nested as tn
+        monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
+
+        mw = BusyMiddleware(
+            ["Tap@snap", "Tap/apps/prometheus@snap"],
+            busy=["Tap/apps/prometheus@snap"], busy_for=99,
+        )
+        survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
+        assert survivors == ["Tap/apps/prometheus@snap"]
+        assert mw.snapshots == ["Tap/apps/prometheus@snap"]
+
+    def test_the_automounts_are_released_before_deleting(self, monkeypatch):
+        import truecloud_nested as tn
+        order = []
+        monkeypatch.setattr(tn, "release_snapdirs",
+                            lambda name, **k: order.append(("release", name)) or [])
+        mw = FakeMiddleware(["Tap@snap"])
+        real = mw.call_sync
+
+        def spy(method, *args):
+            order.append((method, args[0] if args else None))
+            return real(method, *args)
+
+        mw.call_sync = spy
+        tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
+        assert order[0] == ("release", "snap"), order
+
+
+class TestSidecarSurvivesAnIncompleteSweep:
+    def test_the_sidecar_is_KEPT_when_snapshots_could_not_be_deleted(
+        self, tmp_path, monkeypatch
+    ):
+        # It is the ONLY record those snapshots exist. Removing it orphans them
+        # permanently -- which is exactly what happened on the real box.
+        import truecloud_nested as tn
+
+        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
+        monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
+        root = tn.staging_root_for("cloud_backup-5")
+        os.makedirs(root, exist_ok=True)
+        with open(sidecar_for(root), "w", encoding="utf-8") as fh:
+            fh.write("Tap@snap")
+
+        mw = BusyMiddleware(["Tap@snap", "Tap/apps/prometheus@snap"],
+                            busy=["Tap/apps/prometheus@snap"], busy_for=99)
+        monkeypatch.setattr(tn, "delete_snapshot_tree",
+                            lambda m, s, logger=None: ["Tap/apps/prometheus@snap"])
+
+        tn.cleanup_task(mw, "cloud_backup-5")
+        assert os.path.exists(sidecar_for(root)), (
+            "sidecar removed despite survivors — they are now orphaned forever"
+        )
+
+    def test_the_sidecar_is_removed_on_a_clean_sweep(self, tmp_path, monkeypatch):
+        import truecloud_nested as tn
+
+        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
+        root = tn.staging_root_for("cloud_backup-5")
+        os.makedirs(root, exist_ok=True)
+        with open(sidecar_for(root), "w", encoding="utf-8") as fh:
+            fh.write("Tap@snap")
+
+        monkeypatch.setattr(tn, "delete_snapshot_tree", lambda m, s, logger=None: [])
+        tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
+        assert not os.path.exists(sidecar_for(root))
+
+
+class TestTheSidecarCarriesEveryPendingTree:
+    """The sidecar holds a LIST, and that is a bug fix, not a generalisation.
+
+    It used to hold ONE snapshot. So a run that reclaimed an older tree, FAILED to
+    finish reclaiming it, and then recorded its own snapshot would **overwrite the only
+    record of the survivor** — orphaning it permanently, via the exact code written to
+    prevent orphans.
+
+    Observed live: a snapshot survived one run; the next run's reclaim also failed
+    (ZFS's 300s automount window had not elapsed, because the two runs were minutes
+    apart); the record was overwritten; the snapshot was orphaned for good.
+    """
+
+    def test_round_trips_a_list(self, tmp_path):
+        import truecloud_nested as tn
+        root = str(tmp_path / "cloud_backup-5")
+        tn._write_sidecar(root, ["Tap@a", "Tap@b"])
+        assert tn._read_sidecar(root) == ["Tap@a", "Tap@b"]
+
+    def test_reads_the_old_single_line_format(self, tmp_path):
+        # Boxes upgrading from an older version have a one-line sidecar on disk.
+        import truecloud_nested as tn
+        root = str(tmp_path / "cloud_backup-5")
+        os.makedirs(os.path.dirname(sidecar_for(root)), exist_ok=True)
+        with open(sidecar_for(root), "w", encoding="utf-8") as fh:
+            fh.write("Tap@legacy")
+        assert tn._read_sidecar(root) == ["Tap@legacy"]
+
+    def test_a_failed_reclaim_is_carried_forward_not_overwritten(
+        self, tmp_path, monkeypatch
+    ):
+        # THE bug. stage_nested reclaims an old tree, cannot finish, then records its
+        # own snapshot -- the survivor must still be in the sidecar afterwards.
+        import truecloud_nested as tn
+
+        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
+        root = tn.staging_root_for("cloud_backup-5")
+        os.makedirs(os.path.dirname(root), exist_ok=True)
+        tn._write_sidecar(root, ["Tap@old"])
+
+        # The reclaim of Tap@old leaves one snapshot behind (still busy).
+        monkeypatch.setattr(
+            tn, "delete_snapshot_tree",
+            lambda m, s, logger=None: ["Tap/apps/x@old"] if s == "Tap@old" else [],
+        )
+        stub_core(monkeypatch, tn, plan=([("/src", root)], []))
+
+        tn.stage_nested(FakeMiddleware(), "/mnt/Tap", "Tap@new", "Tap", "/mnt/Tap",
+                        "cloud_backup-5", DATASETS)
+
+        recorded = tn._read_sidecar(root)
+        assert "Tap/apps/x@old" in recorded, (
+            "the failed reclaim's survivor was dropped — orphaned forever"
+        )
+        assert "Tap@new" in recorded, "our own snapshot must also be recorded"
+
+    def test_cleanup_sweeps_every_pending_tree_and_records_only_survivors(
+        self, tmp_path, monkeypatch
+    ):
+        import truecloud_nested as tn
+
+        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
+        root = tn.staging_root_for("cloud_backup-5")
+        os.makedirs(root, exist_ok=True)
+        tn._write_sidecar(root, ["Tap@old", "Tap@new"])
+
+        swept = []
+
+        def fake_delete(m, s, logger=None):
+            swept.append(s)
+            return ["Tap/apps/x@new"] if s == "Tap@new" else []
+
+        monkeypatch.setattr(tn, "delete_snapshot_tree", fake_delete)
+        tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
+
+        assert swept == ["Tap@old", "Tap@new"], "both pending trees must be swept"
+        # Only the SURVIVOR is written back -- re-recording Tap@old would make every
+        # future run re-sweep a tree that is already gone.
+        assert tn._read_sidecar(root) == ["Tap/apps/x@new"]
+
+    def test_a_fully_clean_sweep_removes_the_sidecar(self, tmp_path, monkeypatch):
+        import truecloud_nested as tn
+
+        monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
+        root = tn.staging_root_for("cloud_backup-5")
+        os.makedirs(root, exist_ok=True)
+        tn._write_sidecar(root, ["Tap@a", "Tap@b"])
+        monkeypatch.setattr(tn, "delete_snapshot_tree", lambda m, s, logger=None: [])
+
+        tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
+        assert not os.path.exists(sidecar_for(root))
+
+    def test_cleanup_all_reports_each_pending_snapshot_on_its_own_line(self, tmp_path):
+        # It formats them for a human during uninstall. A list rendered into an
+        # f-string would print "['Tap@a', 'Tap@b']" at them.
+        import truecloud_nested as tn
+        root = str(tmp_path / "cloud_backup-5")
+        tn._write_sidecar(root, ["Tap@a", "Tap@b"])
+
+        lines, _errors = tn.cleanup_all(
+            base=str(tmp_path),
+            glob_fn=lambda _p: [sidecar_for(root)],
+            mounts_file=os.devnull,
+        )
+        notes = [ln for ln in lines if "left snapshot" in ln]
+        assert len(notes) == 2
+        assert "'Tap@a'" in notes[0] and "'Tap@b'" in notes[1]
+        assert "[" not in "".join(notes)
+
+
+class TestGarbageCollectorSelection:
+    """`stale_snapshot_names` DELETES DATA on a name match.
+
+    A name match is a weaker claim than a recorded fact, so every way it could be wrong
+    is a test. It exists because the sidecar — which IS a recorded fact — lives in /run,
+    which is tmpfs: a reboot mid-backup destroys it and orphans a 250-snapshot tree with
+    nothing left pointing at it. This is the only thing that would ever find those.
+    """
+
+    import datetime as _dt
+    NOW = _dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=_dt.UTC)
+    CURRENT = "Tap@cloud_backup-5-20260714115900"        # 1 minute ago
+    OLD = "Tap/apps/x@cloud_backup-5-20260713030000"     # ~33 hours ago
+
+    def collect(self, names, **kw):
+        import truecloud_nested as tn
+        return tn.stale_snapshot_names(
+            "cloud_backup-5", self.CURRENT, names, self.NOW, **kw
+        )
+
+    def test_it_collects_our_own_leftovers(self):
+        assert self.collect([self.OLD]) == [self.OLD]
+
+    def test_it_NEVER_touches_the_current_run(self):
+        # Both the parent and its children share the current snapname.
+        names = [self.CURRENT, "Tap/apps/x@cloud_backup-5-20260714115900"]
+        assert self.collect(names) == []
+
+    def test_it_NEVER_touches_a_periodic_snapshot(self):
+        assert self.collect(["Tap/apps/x@auto-2026-07-13_03-00"]) == []
+
+    def test_it_NEVER_touches_a_human_made_snapshot(self):
+        assert self.collect(["Tap@before-i-broke-everything"]) == []
+
+    def test_it_NEVER_touches_another_TASK(self):
+        # cloud_backup-5 must not match cloud_backup-50. This is why the prefix
+        # carries the trailing dash.
+        assert self.collect(["Tap/apps/x@cloud_backup-50-20260713030000"]) == []
+        assert self.collect(["Tap/apps/x@cloud_backup-7-20260713030000"]) == []
+
+    def test_it_NEVER_touches_a_one_time_backup(self):
+        assert self.collect(["Tap@cloud_backup-onetime-20260713030000"]) == []
+
+    def test_it_NEVER_touches_a_snapshot_that_is_MOUNTED(self):
+        # An in-flight run pins its own snapshots. This — not the age heuristic — is
+        # what actually protects a concurrent backup.
+        assert self.collect([self.OLD], in_use={self.OLD}) == []
+
+    def test_it_NEVER_touches_a_snapshot_younger_than_the_minimum_age(self):
+        # Covers the seconds-long window between `zfs snapshot -r` and the mounts
+        # appearing, when a live run's snapshots look exactly like garbage.
+        young = "Tap/apps/x@cloud_backup-5-20260714113000"   # 30 minutes ago
+        assert self.collect([young]) == []
+        assert self.collect([young], min_age=60) == [young]
+
+    def test_a_name_it_cannot_parse_is_left_alone(self):
+        assert self.collect(["Tap@cloud_backup-5-not-a-timestamp"]) == []
+        assert self.collect(["Tap@cloud_backup-5-"]) == []
+
+    def test_a_realistic_mixed_pool(self):
+        names = [
+            self.CURRENT,                                       # ours, running
+            "Tap/apps/x@cloud_backup-5-20260714115900",         # ours, running (child)
+            self.OLD,                                           # ours, orphaned  <-
+            "Tap/apps/y@cloud_backup-5-20260712030000",         # ours, orphaned  <-
+            "Tap/apps/x@auto-2026-07-13_03-00",                 # periodic
+            "Tap/apps/x@cloud_backup-7-20260713030000",         # another task
+            "Tap@manual-keepme",                                # human
+        ]
+        assert sorted(self.collect(names)) == sorted(
+            [self.OLD, "Tap/apps/y@cloud_backup-5-20260712030000"]
+        )
+
+
+class TestMountedSnapshots:
+    def test_it_reads_snapshot_names_out_of_the_mount_table(self, tmp_path):
+        import truecloud_nested as tn
+        mounts = tmp_path / "mounts"
+        mounts.write_text(
+            "tmpfs /run tmpfs rw 0 0\n"
+            "Tap/apps/x@snap1 /run/truecloud-nested/t/apps/x zfs ro 0 0\n"
+            "Tap/apps/y@snap1 /mnt/Tap/apps/y/.zfs/snapshot/snap1 zfs ro 0 0\n"
+            "Tap/live /mnt/Tap/live zfs rw 0 0\n"
+        )
+        live = tn.mounted_snapshots(str(mounts))
+        assert live == {"Tap/apps/x@snap1", "Tap/apps/y@snap1"}
+        assert "Tap/live" not in live      # a live dataset is not a snapshot
+
+
+class TestGarbageCollectorExecution:
+    def test_it_deletes_the_stale_ones_and_nothing_else(self, monkeypatch, tmp_path):
+        import datetime as dt
+        import truecloud_nested as tn
+
+        mounts = tmp_path / "mounts"
+        mounts.write_text("")
+        now = dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC)
+
+        mw = FakeMiddleware([
+            "Tap@cloud_backup-5-20260714115900",             # current run
+            "Tap/apps/x@cloud_backup-5-20260713030000",      # orphan   <-
+            "Tap/apps/x@auto-2026-07-13_03-00",              # periodic
+            "Tap/apps/x@cloud_backup-7-20260713030000",      # other task
+        ])
+        remaining = tn.gc_stale_snapshots(
+            mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
+            now=now, mounts_file=str(mounts),
+        )
+        assert remaining == []
+        assert mw.snapshots == [
+            "Tap@cloud_backup-5-20260714115900",
+            "Tap/apps/x@auto-2026-07-13_03-00",
+            "Tap/apps/x@cloud_backup-7-20260713030000",
+        ]
+
+    def test_a_busy_orphan_is_reported_not_swallowed(self, monkeypatch, tmp_path):
+        import datetime as dt
+        import truecloud_nested as tn
+
+        mounts = tmp_path / "mounts"
+        mounts.write_text("")
+        now = dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC)
+        orphan = "Tap/apps/x@cloud_backup-5-20260713030000"
+
+        mw = BusyMiddleware(
+            ["Tap@cloud_backup-5-20260714115900", orphan],
+            busy=[orphan], busy_for=99,
+        )
+        remaining = tn.gc_stale_snapshots(
+            mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
+            now=now, mounts_file=str(mounts),
+        )
+        assert remaining == [orphan]
+
+    def test_it_collects_NOTHING_when_the_query_fails(self, tmp_path):
+        # Cannot enumerate => cannot know what is ours => delete nothing.
+        import datetime as dt
+        import truecloud_nested as tn
+
+        mounts = tmp_path / "mounts"
+        mounts.write_text("")
+
+        class Broken(FakeMiddleware):
+            def call_sync(self, method, *args):
+                if method == "zfs.snapshot.query":
+                    raise RuntimeError("middleware is having a day")
+                return super().call_sync(method, *args)
+
+        assert tn.gc_stale_snapshots(
+            Broken(["Tap/apps/x@cloud_backup-5-20260713030000"]),
+            "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
+            now=dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC),
+            mounts_file=str(mounts),
+        ) == []
@@ -10,7 +10,8 @@ import re

 import pytest

-WORKFLOWS = os.path.join(os.path.dirname(__file__), "..", ".github", "workflows")
+ROOT = os.path.join(os.path.dirname(__file__), "..")
+WORKFLOWS = os.path.join(ROOT, ".github", "workflows")


 def workflow_files():
@@ -84,11 +85,77 @@ class TestBothForges:
        assert "if: ${{ contains(github.server_url, 'github.com') }}" in src
        assert "if: ${{ !contains(github.server_url, 'github.com') }}" in src

-    def test_compat_files_an_issue_on_each_forge(self):
+    def test_compat_files_its_report_through_ONE_implementation(self):
+        # It used to be two near-identical shell steps, one per forge. Two copies of
+        # "find the issue, decide whether to comment, post it" is two chances to drift,
+        # and the Gitea one duplicated an issue for real.
        with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh:
            src = fh.read()
-        assert "file a bug report (GitHub)" in src
-        assert "file a bug report (Gitea)" in src
+        assert "tools/compat_publish.py" in src
+        assert "file a bug report (GitHub)" not in src
+        assert "file a bug report (Gitea)" not in src
+
+
+class TestTheBotDoesNotSpam:
+    """It left 11 identical 3,000-character comments on one issue in a single day.
+
+    A bot that repeats itself daily gets muted — and then the next REAL finding is
+    scrolled past, which defeats the entire reason for building it.
+    """
+
+    def publisher(self):
+        with open(os.path.join(ROOT, "tools", "compat_publish.py"), encoding="utf-8") as fh:
+            return fh.read()
+
+    def test_it_compares_a_fingerprint_before_saying_anything(self):
+        src = self.publisher()
+        assert "extract_fingerprint" in src
+        assert "staying quiet" in src
+
+    def test_the_body_is_edited_in_place_not_appended_to(self):
+        src = self.publisher()
+        assert '"PATCH"' in src, "the issue body must be updated, not commented onto"
+
+    def test_it_closes_the_issue_when_everything_is_fixed(self):
+        src = self.publisher()
+        assert '"state": "closed"' in src
+
+    def test_the_matrix_refresh_opens_a_PR_rather_than_pushing_to_main(self):
+        # An unattended push to main from CI is exactly what the release barrier exists
+        # to prevent: a bot that can move main can move it somewhere nobody looked.
+        #
+        # Checked against CODE, not comments — the step's own commentary explains what
+        # it replaced, and that mention must not read as the thing itself.
+        with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh:
+            src = fh.read()
+        code = "\n".join(
+            ln for ln in src.splitlines() if not ln.lstrip().startswith("#")
+        )
+        assert "/pulls" in code, "the matrix refresh must open a PR"
+        assert "HEAD:main" not in code, "CI still pushes straight to main"
+
+    def test_the_matrix_PR_targets_the_CANONICAL_forge_not_the_mirror(self):
+        # GitHub is a one-way mirror: a PR merged there would be silently clobbered by
+        # the next `fleet-repos mirror` push from Gitea. A bot opening PRs against a
+        # mirror is a bot doing nothing, slowly.
+        with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh:
+            src = fh.read()
+        i = src.index("refresh the README matrix")
+        step = src[i:i + 400]
+        assert "!contains(github.server_url, 'github.com')" in step, (
+            "the matrix PR must be opened on Gitea (canonical), not GitHub (mirror)"
+        )
+
+    def test_the_workflow_has_the_permissions_its_steps_actually_need(self):
+        # It shipped with `contents: read` while the step pushed a branch and opened a
+        # PR — it would have died with a 403 on the first scheduled run, and I would
+        # have had a bot that silently never worked.
+        with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh:
+            src = fh.read()
+        perms = src[src.index("permissions:"):src.index("jobs:")]
+        assert "contents: write" in perms, "pushing a branch needs contents: write"
+        assert "pull-requests: write" in perms, "opening a PR needs pull-requests: write"
+        assert "issues: write" in perms


 class TestCompatCannotSilentlyPass:
@@ -43,6 +43,7 @@ from __future__ import annotations

 import argparse
 import ast
+import hashlib
 import json
 import os
 import sys
@@ -892,6 +893,90 @@ def render_markdown(rows: list[dict]) -> str:
    return "\n".join(out) + "\n" + _LEGEND


+FINGERPRINT = "<!-- compat-fingerprint:"
+
+
+def fingerprint(rows: list[dict]) -> str:
+    """A stable digest of WHAT IS BROKEN, and nothing else.
+
+    The bug report must be updated when the findings change and stay silent when they
+    do not. Without this the workflow commented on every run -- it left **11 identical
+    3,000-character comments** on one issue in a single day, which is not a warning
+    system, it is a mute button with extra steps.
+
+    Deliberately excludes anything that moves on its own: the matrix's `ok` rows, the
+    hardware-verified column, and the exact TrueNAS point-release (`TS-25.10.4` ->
+    `TS-25.10.5` is not news). Only the broken (ref, module, problem-id) triples count.
+    """
+    findings = sorted(
+        (r["ref"], mod, p["id"])
+        for r in rows
+        for mod, m in r["modules"].items()
+        if is_broken(m)
+        for p in m["problems"]
+    )
+    return hashlib.sha256(repr(findings).encode()).hexdigest()[:16]
+
+
+def extract_fingerprint(body: str) -> str | None:
+    """The fingerprint a previous run left in the issue body, if any."""
+    if not body:
+        return None
+    i = body.find(FINGERPRINT)
+    if i == -1:
+        return None
+    return body[i + len(FINGERPRINT):].split("-->", 1)[0].strip() or None
+
+
+def render_issue(rows: list[dict]) -> str:
+    """The bug report body: what is broken, why it matters, and nothing else up front.
+
+    Short by design. The full matrix and the healthy versions go in a fold -- somebody
+    opening this wants to know what broke and whether it can hurt them, not to re-read
+    a table they can see in the README.
+    """
+    broken = [r for r in rows if any(is_broken(m) for m in r["modules"].values())]
+
+    out = [
+        "`tools/compat.py` checks what this patch assumes about middlewared against "
+        "iXsystems' actual source, every day. Those assumptions no longer hold on the "
+        "versions below.",
+        "",
+        "**This does not break anyone today.** `apply.sh` re-checks on every boot and "
+        "**declines to apply** a module whose assumptions fail, so TrueNAS is left "
+        "stock rather than half-patched. The cost is the module's feature, not a "
+        "broken backup.",
+        "",
+    ]
+
+    for r in broken:
+        out.append(f"### `{r['ref']}`")
+        out.append("")
+        for mod, m in sorted(r["modules"].items()):
+            if not is_broken(m):
+                continue
+            out.append(f"**{mod}**")
+            out.append("")
+            for p in m["problems"]:
+                out.append(f"- {p['detail']}")
+                out.append(f"  <br><sub>{p['why']}</sub>")
+            out.append("")
+
+    out += [
+        "<details><summary>Full support matrix</summary>",
+        "",
+        render_markdown(rows),
+        "</details>",
+        "",
+        "_Filed and kept up to date by "
+        "[`compat.yml`](.github/workflows/compat.yml). It edits this body when the "
+        "findings change, and stays quiet when they do not._",
+        "",
+        f"{FINGERPRINT} {fingerprint(rows)} -->",
+    ]
+    return "\n".join(out)
+
+
 def render_matrix(rows: list[dict]) -> str:
    """A support table.

@@ -0,0 +1,148 @@
+#!/usr/bin/env python3
+"""Keep ONE bug report in sync with what compat.py currently finds.
+
+WHY THIS IS NOT JUST "POST A COMMENT"
+-------------------------------------
+The first version commented on every run that found a break. In one day it left
+**11 identical 3,000-character comments** on the same issue. That is not a warning
+system; it is a mute button with extra steps. The next real finding would have been
+scrolled past, which defeats the entire point of building it.
+
+So:
+
+  * **The issue body is the current truth.** It is edited in place, never appended to.
+  * **Comments are a changelog of CHANGES.** A run whose findings are identical to the
+    last one says nothing at all -- no comment, no edit, no notification.
+  * A fingerprint of the findings (broken ref/module/problem triples only) is embedded
+    in the body. It deliberately ignores things that move on their own -- healthy rows,
+    the hardware-verified column, TrueNAS point releases -- so `TS-25.10.4` becoming
+    `TS-25.10.5` is not news, and does not wake anybody up.
+
+  * When everything is fixed, the issue is **closed** with a comment saying so.
+
+Works against GitHub and Gitea, which differ only in the auth header and the issue
+list URL. One implementation, so the two cannot drift.
+
+    python3 tools/compat_publish.py --api <url> --token <tok> --matrix /tmp/matrix.json
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import sys
+import urllib.error
+import urllib.request
+
+sys.path.insert(0, __file__.rsplit("/", 1)[0])
+
+from compat import (  # noqa: E402
+    extract_fingerprint,
+    fingerprint,
+    is_broken,
+    render_issue,
+)
+
+TITLE = "TrueNAS compatibility: the patch's assumptions no longer hold"
+
+
+def _call(url, token, method="GET", data=None):
+    req = urllib.request.Request(
+        url, method=method,
+        headers={
+            # Gitea wants `token <t>`; GitHub accepts `Bearer <t>`. GitHub also
+            # accepts `token <t>`, so one header serves both.
+            "Authorization": f"token {token}",
+            "Content-Type": "application/json",
+            "Accept": "application/vnd.github+json",
+        },
+        data=json.dumps(data).encode() if data else None,
+    )
+    with urllib.request.urlopen(req) as r:  # noqa: S310
+        return json.load(r) if r.length != 0 else {}
+
+
+def find_issue(api, token, title):
+    """The LOWEST-numbered issue with this title, open or closed.
+
+    Lowest, not "whichever the API returns first": two issues with the same title
+    existed once (an earlier version put the ref list in the title, so the identity
+    changed whenever that set changed), and an order-dependent pick would alternate
+    between them -- reopening one while commenting on the other.
+    """
+    issues = _call(f"{api}/issues?state=all&per_page=100", token)
+    mine = [
+        i for i in issues
+        if i.get("title") == title and "pull_request" not in i   # GitHub lists PRs here
+    ]
+    return min(mine, key=lambda i: i["number"]) if mine else None
+
+
+def main(argv):
+    ap = argparse.ArgumentParser(description=__doc__.split("\n")[0])
+    ap.add_argument("--api", required=True, help="…/repos/<owner>/<repo>")
+    ap.add_argument("--token", required=True)
+    ap.add_argument("--matrix", required=True, help="compat.py --matrix --json output")
+    args = ap.parse_args(argv[1:])
+
+    with open(args.matrix, encoding="utf-8") as fh:
+        rows = json.load(fh)
+
+    broken = [r for r in rows if any(is_broken(m) for m in r["modules"].values())]
+    issue = find_issue(args.api, args.token, TITLE)
+
+    # ── everything is healthy ────────────────────────────────────────────────
+    if not broken:
+        if issue and issue["state"] == "open":
+            _call(f"{args.api}/issues/{issue['number']}/comments", args.token, "POST",
+                  {"body": "All of the patch's assumptions hold again on every "
+                           "checked TrueNAS version. Closing."})
+            _call(f"{args.api}/issues/{issue['number']}", args.token, "PATCH",
+                  {"state": "closed"})
+            print(f"closed #{issue['number']} — nothing is broken any more")
+        else:
+            print("nothing broken; no open report to close")
+        return 0
+
+    body = render_issue(rows)
+    want = fingerprint(rows)
+
+    # ── nothing to file yet ──────────────────────────────────────────────────
+    if issue is None:
+        made = _call(f"{args.api}/issues", args.token, "POST",
+                     {"title": TITLE, "body": body})
+        print(f"filed #{made['number']}")
+        return 0
+
+    have = extract_fingerprint(issue.get("body") or "")
+    n = issue["number"]
+
+    # ── the findings are UNCHANGED: say nothing ──────────────────────────────
+    #
+    # This is the whole point. A daily "still broken, same as yesterday" comment is
+    # what taught everyone to ignore the last one.
+    if have == want and issue["state"] == "open":
+        print(f"#{n} is already current ({want}) — staying quiet")
+        return 0
+
+    _call(f"{args.api}/issues/{n}", args.token, "PATCH", {"body": body, "state": "open"})
+
+    if have != want:
+        refs = ", ".join(f"`{r['ref']}`" for r in broken)
+        note = (
+            "The findings changed — the report above has been updated.\n\n"
+            f"Currently broken on: {refs}."
+            if have else
+            "This report is now kept up to date automatically: the body above always "
+            "reflects the current findings, and a comment is only added when they "
+            "change."
+        )
+        _call(f"{args.api}/issues/{n}/comments", args.token, "POST", {"body": note})
+        print(f"updated #{n}: {have} -> {want}")
+    else:
+        print(f"reopened #{n}")
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv))
@@ -3,7 +3,7 @@

 set -euo pipefail

-VERSION="0.6.0"
+VERSION="0.6.1"

 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _HOOK_COMMENT='TrueCloud provider patch (S3/B2)'
@@ -19,7 +19,7 @@

 set -euo pipefail

-VERSION="0.6.0"
+VERSION="0.6.1"

 PATCH_DIR="$(cd "$(dirname "$0")" && pwd)"
 _PREV_FILE="$PATCH_DIR/.update_previous"
@@ -123,7 +123,7 @@ cd "$PATCH_DIR"

 if ! git rev-parse --git-dir >/dev/null 2>&1; then
    echo "ERROR: $PATCH_DIR is not a git clone — nothing to update." >&2
-    echo "  Re-clone from https://git.onetick.ninja/flan/truenas-truecloud-patch" >&2
+    echo "  Re-clone from https://github.com/sudolulo/truenas-truecloud-patch" >&2
    exit 1
 fi

@@ -136,9 +136,21 @@ fi

 # A dirty tree means someone edited or scp'd files in place; merging over that
 # silently loses their changes, or conflicts halfway through.
-if [ -n "$(git status --porcelain --untracked-files=no)" ]; then
+#
+# CONTENT changes only. A mode-only change (100644 -> 100755) is not somebody's work
+# and must not block an update -- and it is not hypothetical: install.sh chmod +x's
+# these very scripts, so on any version where git recorded one as 100644, INSTALLING
+# dirtied the checkout and update.sh then refused to run. Install once, and updating
+# was blocked forever, with an error telling the user to `git checkout -- .` (which
+# merely undoes the exec bit so the next install can re-dirty it). A real box sat on
+# an old version for exactly this reason.
+#
+# `git diff --numstat` reports "0  0  file" for a mode-only change, so anything with a
+# nonzero insert or delete count is a genuine edit.
+_dirty=$(git diff --numstat HEAD -- . | awk '$1 != 0 || $2 != 0 { print $3 }')
+if [ -n "$_dirty" ]; then
    echo "ERROR: the working tree has uncommitted changes:" >&2
-    git status --short --untracked-files=no >&2
+    printf '  M %s\n' $_dirty >&2
    echo "" >&2
    echo "  Refusing to update over them. Commit, stash, or discard them first:" >&2
    echo "    git -C $PATCH_DIR checkout -- ." >&2
Author	SHA1	Message	Date
flan	7cc0826c2c	The matrix bot would never have worked: wrong permissions, wrong forge CI / shell (shellcheck + syntax) (push) Successful in 9s Details CI / python 3.11 (push) Successful in 13s Details CI / python 3.13 (push) Successful in 16s Details CI / python 3.12 (push) Successful in 16s Details TrueNAS compatibility / compat (push) Failing after 6s Details Release / release (push) Successful in 16s Details Two bugs, both of which would have failed silently on the first scheduled run: - permissions were while the step pushes a branch and opens a PR. It would have died with a 403 and I would have had a bot that never worked. - it opened the PR on GITHUB, which is a one-way MIRROR. A PR merged there would be clobbered by the next fleet-repos mirror push from Gitea. A bot opening PRs against a mirror is a bot doing nothing, slowly. Now: contents+pull-requests write, and the PR is opened on Gitea (canonical) via its API. One long-lived PR, force-pushed in place -- a daily PR is the same mistake as a daily comment, wearing a hat.	2026-07-13 20:28:30 +00:00
flan	82084b6806	The bug-report bot was spamming; make it say something only when there is something to say CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 14s Details CI / python 3.12 (push) Successful in 17s Details CI / python 3.13 (push) Successful in 17s Details TrueNAS compatibility / compat (push) Successful in 11s Details It commented on every run that found a break. In one day it left ELEVEN identical 3,000-character comments on the same issue. That is not a warning system, it is a mute button with extra steps -- and the next real finding would have been scrolled past, which defeats the entire reason for building it. Now: the issue BODY is the current truth, edited in place. COMMENTS are a changelog of changes. A fingerprint of the findings (broken ref/module/problem triples only) is embedded in the body; a run whose findings match it says nothing at all. It closes the issue when everything is fixed. The fingerprint deliberately ignores anything that moves on its own -- healthy rows, the hardware-verified column, TrueNAS point releases -- so TS-25.10.4 becoming TS-25.10.5 is not news and does not wake anybody up. Also: - The two near-identical per-forge shell steps are gone, replaced by one tested implementation (tools/compat_publish.py). Two copies of 'find the issue, decide whether to comment' is two chances to drift, and the Gitea one duplicated an issue for real. - The README matrix refresh now opens a PULL REQUEST instead of pushing straight to main from CI. An unattended push to main is exactly what the release barrier exists to prevent: a bot that can move main can move it somewhere nobody looked.	2026-07-13 20:20:35 +00:00
flan	c6b252ac6b	release v0.6.1 CI / shell (shellcheck + syntax) (push) Successful in 11s Details CI / python 3.11 (push) Successful in 13s Details CI / python 3.12 (push) Successful in 14s Details CI / python 3.13 (push) Successful in 13s Details TrueNAS compatibility / compat (push) Successful in 11s Details Release / release (push) Successful in 14s Details	2026-07-13 19:59:38 +00:00
flan	841e0364fd	CHANGELOG: repair a section spliced into the middle of a bullet, and guard it CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 16s Details CI / python 3.12 (push) Successful in 15s Details CI / python 3.13 (push) Successful in 14s Details An edit matched the literal '## Unreleased' inside a backticked phrase in a prose bullet and spliced a whole new section into the middle of it, splitting the sentence in half. The release body IS this file, so that would have shipped to every user. Tests now assert: no empty version section, versions descend, no heading is indented inside a list item, and every bullet's bold phrases are balanced (ignoring code spans -- 'args, *kwargs' is a literal, not markup).	2026-07-13 19:59:35 +00:00
flan	0d04c2cd1c	Collect orphaned snapshots by name: the sidecar lives in tmpfs CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 13s Details CI / python 3.12 (push) Successful in 16s Details CI / python 3.13 (push) Successful in 16s Details A reboot mid-backup orphaned the entire tree, permanently. The sidecar is the record of which snapshots a run pinned -- and /run is tmpfs. A reboot or crash between the recursive snapshot and its cleanup destroyed that record, leaving one snapshot per descendant dataset (250+ on a real pool) with nothing pointing at them. Nothing would ever have found them. gc_stale_snapshots() identifies leftovers by NAME, so it works when the record is gone. It runs after the sidecar reclaim -- the recorded path stays authoritative and the collector only mops up what the record lost. It deletes data on a name match, which is a weaker claim than a recorded fact, so the selection is a pure function with the harshest tests here. A snapshot is collected only if the name is exactly <dataset>@<task>-<YYYYMMDDHHMMSS>, it is not the current run's, NOTHING IS MOUNTED FROM IT (this, not the age guard, is what protects a concurrent backup), and it is over an hour old. Checked against the real pool: of 4728 snapshots including 2341 periodic ones, it selects exactly the orphans of the task being run and nothing else.	2026-07-13 19:56:32 +00:00
flan	2b8ef107f7	The sidecar must carry EVERY pending tree, not just the newest CI / shell (shellcheck + syntax) (push) Successful in 9s Details CI / python 3.12 (push) Successful in 13s Details CI / python 3.13 (push) Successful in 17s Details CI / python 3.11 (push) Successful in 15s Details TrueNAS compatibility / compat (push) Successful in 11s Details Release / release (push) Successful in 15s Details Found live, in the code written to prevent exactly this. The sidecar held ONE snapshot. So a run that reclaimed an older tree, failed to finish reclaiming it, and then recorded its own snapshot OVERWROTE the only record of the survivor -- orphaning it permanently. Observed: job 24 left one snapshot busy and kept the sidecar (correct). Job 46 reclaimed it, hit ZFS's 300s automount window (the runs were minutes apart), left it behind again, and then wrote its own snapshot over the record. Permanent orphan, created by the safety net. The sidecar is now a list. stage_nested carries forward whatever a reclaim could not delete; cleanup_task sweeps every pending tree and writes back only the survivors. cleanup_all reports them one per line instead of formatting a list into an f-string at the user during uninstall. Job 46 also confirms the automount fix itself: it swept all 256 of its own snapshots with no straggler.	2026-07-13 19:35:44 +00:00
flan	b50567e9a7	A few snapshots leaked on every nested run, forever CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 12s Details CI / python 3.12 (push) Successful in 15s Details CI / python 3.13 (push) Successful in 17s Details TrueNAS compatibility / compat (push) Successful in 13s Details Release / release (push) Successful in 14s Details Found on real hardware: a 256-snapshot backup of /mnt/Tap swept 253 and left 3 with 'dataset is busy'. ZFS AUTOMOUNTS <dataset>/.zfs/snapshot/<snap> when it is read, and keeps it mounted for zfs_expire_snapshot seconds (300 default) after the last access. teardown() unmounts OUR bind mounts but not the automount underneath, so zfs destroy refuses for exactly the datasets restic read most recently. cleanup_task() then removed the sidecar anyway -- destroying the only record those snapshots existed. Nothing would ever have reclaimed them. - release_snapdirs() unmounts ZFS's own automounts (deepest first) before deleting. - delete_snapshot_tree() retries the transient busy and RETURNS what it could not delete, instead of swallowing it. - The sidecar is removed only on a confirmed-clean sweep -- including on the staging-failure path, which used to remove it before the caller swept. A sidecar left behind when the tree is gone costs one no-op delete; a sidecar removed while the tree exists is unrecoverable.	2026-07-13 19:18:22 +00:00
flan	1b2407f6e2	compat: dedup the bug report deterministically (lowest issue number wins) CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 16s Details CI / python 3.12 (push) Successful in 18s Details CI / python 3.13 (push) Successful in 18s Details TrueNAS compatibility / compat (push) Successful in 10s Details Two issues with the same title already existed -- the old title embedded the list of broken refs, so the issue's identity changed whenever that set changed. With an order-dependent pick the bot would alternate between them, reopening one and commenting on the other. Lowest number is stable regardless of how the API sorts.	2026-07-13 19:08:27 +00:00
flan	ab0b66c47d	compat: the bug-report title must be stable across ref-set changes CI / shell (shellcheck + syntax) (push) Successful in 10s Details CI / python 3.11 (push) Successful in 15s Details CI / python 3.12 (push) Successful in 17s Details CI / python 3.13 (push) Successful in 18s Details TrueNAS compatibility / compat (push) Successful in 10s Details The title embedded the list of broken refs, so the issue's identity changed whenever that set changed -- and it did: when the async/sync port briefly made 26 look green, the next run filed a SECOND issue for 'master' alone. A bot that spawns duplicates gets muted, and then it is not a warning system any more. The title is now fixed; the refs live in the body, which gets updated in place.	2026-07-13 19:02:57 +00:00
flan	469a2e4651	Installing the patch permanently blocked updating it CI / shell (shellcheck + syntax) (push) Successful in 9s Details CI / python 3.11 (push) Successful in 14s Details CI / python 3.12 (push) Successful in 16s Details CI / python 3.13 (push) Successful in 17s Details TrueNAS compatibility / compat (push) Successful in 11s Details Release / release (push) Successful in 14s Details install.sh chmod +x's update.sh, and git recorded update.sh as 100644 -- so the chmod was a TRACKED modification, and update.sh refuses to run over a dirty tree. Install once and you could never update again. The error even told you to 'git checkout -- .', which just undoes the exec bit so the next install can re-dirty it. Found on the real box, which had been sitting on v0.4.1 for exactly this reason. Fixed on both sides: the scripts install.sh chmods are executable in git (so the chmod is a no-op), and update.sh's dirty check now looks at CONTENT, not mode -- git diff --numstat reports 0 0 for a mode-only change. A test asserts every script in install.sh's chmod loop is already 100755 in git.	2026-07-13 18:58:46 +00:00
flan	518a22d87e	docs: user-facing URLs point at GitHub, the user-facing repo CI / shell (shellcheck + syntax) (push) Successful in 8s Details CI / python 3.11 (push) Successful in 13s Details CI / python 3.12 (push) Successful in 14s Details CI / python 3.13 (push) Successful in 15s Details TrueNAS compatibility / compat (push) Failing after 6s Details Release / release (push) Successful in 14s Details Gitea is canonical for development; GitHub is where users clone from and where the box's read-only checkout points. The install instructions, the re-clone hint and the 'file an issue' link are all read by users, so they name GitHub. docs/releasing.md still names Gitea, because that is a contributor doc about where the code is pushed.	2026-07-13 18:42:44 +00:00