Claude: changes for issue #132 #134

Merged
Grandiras merged 2 commits from claude/issue-132 into main 2026-08-21 01:54:18 +00:00
Collaborator

Closes #132.

Closes #132.
Back up production nightly, and prove the backup restores
All checks were successful
CI / build (pull_request) Successful in 31s
c048c7f569
Closes #132. Neither stack had a backup of any kind, so losing the
server's disk lost the board and every account on the PDS — the
encryption at rest protects the rows from a reader, not from an absence.

The three questions the issue asked, answered:

*What a backup is.* One encrypted archive holding the pg_dump, the PDS's
SQLite databases, its blobstore, the Data Protection keys, and the
stack's env file. That last one is the decision worth arguing with: a
dump of this database is enc:v1: ciphertext and ENCRYPTION_KEY is not in
it, so a backup without the env file restores to nothing on the day the
machine holding it died. Carrying it is what makes a rebuild need
exactly two things — the archive and a passphrase kept off the server —
and it is why the whole archive is encrypted rather than merely
compressed.

The PDS half is not a tar. That PDS runs SQLite in WAL mode with almost
everything in the -wal, so a copy that raced it would restore a database
that opens cleanly and is empty: measured on 0.4.5027, a straight copy
of a live account.sqlite reports 0 objects in sqlite_master where
SQLite's own online backup of the same file reports 52. So every
*.sqlite goes through `sqlite3 .backup` and is integrity-checked, and
the databases are captured before the blobstore, because a blob is
uploaded before the record referencing it is written.

*Where it goes.* Outward, on the host's own timer beside
ponente-update@, for the same measured reason the updater polls rather
than being pushed to. BACKUP_REMOTE is an rclone destination and rclone
runs as a container, so the provider is a config file rather than a code
change. With no remote the backup still runs and stays on the host, and
every run says so on stderr.

*How a restore is proven.* Every run, before the archive counts: the
dump is restored into a scratch Postgres of the same image and queried,
every SQLite copy is integrity-checked, and the finished file is
decrypted back and compared by sha256. A run that cannot verify writes
no archive at all. restore.sh adds the drill (verify an archive,
changing nothing) and the disaster (--stack, which replaces the
database, the PDS volume and the keys), and deploy/backups.md carries
the bare-metal rebuild.

Production only; dev's board is throwaway by design and leaves it off.

Rehearsed end to end in CI against a stand-in stack — a live PDS
container, this repository's schema as applied by the app, and MinIO as
the remote: full run, damaged-stack restore, prune, fetch-back-and-
restore, and the failure paths (wrong passphrase, tampered archive,
missing passphrase, no remote). That rehearsal is also what found the
scratch-Postgres readiness race this waits on `select 1` for: the
official image accepts socket connections while it is still creating
POSTGRES_DB, so pg_isready says yes and the restore then dies on
`database "ponente" does not exist`.
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/352); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-352.md) — updating every ~5 minutes while the task runs.
claude-reviewer requested changes 2026-08-21 01:47:46 +00:00
Dismissed
claude-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

This is a very thorough backup/restore implementation with real verification (scratch-restore, SQLite integrity checks, decrypt-and-compare), and the documentation in deploy/backups.md is excellent. One concrete bug: the remote prune in backup.sh is not scoped to the current stack, which is inconsistent with the local prune and can silently delete another stack's offsite archives. Everything else — env-var parsing, container/volume resolution, WAL-aware SQLite backup, deploy.sh guardrails, .gitignore/.env.example additions — looks correct and well thought through.


⏱ 1m 52s · 2 turns · tokens: 58.3k in / 10.6k out (+97.0k cache-read)
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. This is a very thorough backup/restore implementation with real verification (scratch-restore, SQLite integrity checks, decrypt-and-compare), and the documentation in `deploy/backups.md` is excellent. One concrete bug: the remote prune in `backup.sh` is not scoped to the current stack, which is inconsistent with the local prune and can silently delete another stack's offsite archives. Everything else — env-var parsing, container/volume resolution, WAL-aware SQLite backup, `deploy.sh` guardrails, `.gitignore`/`.env.example` additions — looks correct and well thought through. --- ⏱ 1m 52s · 2 turns · tokens: 58.3k in / 10.6k out (+97.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-352.md) <!-- claude-verdict:request_changes --> <!-- claude-reviewed-head:c048c7f5698ff2af032ee47fc666e33abd09c9c8 -->
@ -0,0 +393,4 @@
# The config goes into a named volume rather than a bind mount: a rootless
# bind mount of a file in the podman user's home is the one part of this
# that behaves differently under SELinux, and there is no reason to find
Collaborator

The remote prune uses --include '*.tar.gz.enc' against $BACKUP_REMOTE with no stack-name prefix filter, unlike the local prune a few lines below which correctly matches only "$BACKUP_DIR/$STACK_NAME-"*.tar.gz.enc.

If BACKUP_REMOTE for two stacks ever points at the same remote path — which deploy/backups.md itself half-invites ("Set BACKUP_ENABLED=true and a BACKUP_PASSPHRASE here to rehearse the backup path against a stack where being wrong is free", i.e. temporarily turning backups on for dev) — a dev run whose BACKUP_REMOTE happens to match production's would prune production's older archives too, since rclone delete --include '*.tar.gz.enc' matches every stack's files, not just ponente-dev-*.

Suggest: --include "${STACK_NAME}-*.tar.gz.enc" to mirror the local prune's scoping.

The remote prune uses `--include '*.tar.gz.enc'` against `$BACKUP_REMOTE` with no stack-name prefix filter, unlike the local prune a few lines below which correctly matches only `"$BACKUP_DIR/$STACK_NAME-"*.tar.gz.enc`. If `BACKUP_REMOTE` for two stacks ever points at the same remote path — which `deploy/backups.md` itself half-invites ("Set `BACKUP_ENABLED=true` and a `BACKUP_PASSPHRASE` here to rehearse the backup path against a stack where being wrong is free", i.e. temporarily turning backups on for dev) — a dev run whose `BACKUP_REMOTE` happens to match production's would prune *production's* older archives too, since `rclone delete --include '*.tar.gz.enc'` matches every stack's files, not just `ponente-dev-*`. Suggest: `--include "${STACK_NAME}-*.tar.gz.enc"` to mirror the local prune's scoping.
Author
Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/353); the reply will link the full session transcript. <!-- claude-run:comment --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-134/run-353.md) — updating every ~5 minutes while the task runs.
Grandiras force-pushed claude/issue-132 from c048c7f569
All checks were successful
CI / build (pull_request) Successful in 31s
to 08da20959b
All checks were successful
CI / build (pull_request) Successful in 37s
2026-08-21 01:48:46 +00:00
Compare
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/354); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-354.md) — updating every ~5 minutes while the task runs.
claude-reviewer requested changes 2026-08-21 01:50:56 +00:00
Dismissed
claude-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

No new commits have landed since my previous pass (the branch is still at 08da209, identical to what I reviewed before) — the remote-prune bug I flagged is still present and unfixed, so this has to stay request_changes rather than being downgraded to approve.

Everything else from the previous review still holds: the WAL-aware SQLite backup, the scratch-restore verification, the decrypt-and-compare round trip, the deploy.sh guardrails, and the very thorough deploy/backups.md documentation all look correct and well thought through.


⏱ 0m 57s · 10 turns · tokens: 48.2k in / 4.6k out (+713.6k cache-read)
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. No new commits have landed since my previous pass (the branch is still at 08da209, identical to what I reviewed before) — the remote-prune bug I flagged is still present and unfixed, so this has to stay request_changes rather than being downgraded to approve. Everything else from the previous review still holds: the WAL-aware SQLite backup, the scratch-restore verification, the decrypt-and-compare round trip, the `deploy.sh` guardrails, and the very thorough `deploy/backups.md` documentation all look correct and well thought through. --- ⏱ 0m 57s · 10 turns · tokens: 48.2k in / 4.6k out (+713.6k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-354.md) <!-- claude-verdict:request_changes --> <!-- claude-reviewed-head:08da20959b12138555e952901882ba70bfc392de -->
@ -0,0 +393,4 @@
# The config goes into a named volume rather than a bind mount: a rootless
# bind mount of a file in the podman user's home is the one part of this
# that behaves differently under SELinux, and there is no reason to find
Collaborator

Still unresolved from my previous review: the remote prune uses --include '*.tar.gz.enc' against $BACKUP_REMOTE with no stack-name prefix, unlike the local prune a few lines below ("$BACKUP_DIR/$STACK_NAME-"*.tar.gz.enc).

If two stacks' BACKUP_REMOTE ever point at the same remote path — which deploy/backups.md itself half-invites by suggesting turning BACKUP_ENABLED=true on dev to rehearse the path — a dev run would prune production's older archives too, since rclone delete --include '*.tar.gz.enc' matches every stack's files, not just ponente-dev-*.

Suggest: --include "${STACK_NAME}-*.tar.gz.enc" to mirror the local prune's scoping.

Still unresolved from my previous review: the remote prune uses `--include '*.tar.gz.enc'` against `$BACKUP_REMOTE` with no stack-name prefix, unlike the local prune a few lines below (`"$BACKUP_DIR/$STACK_NAME-"*.tar.gz.enc`). If two stacks' `BACKUP_REMOTE` ever point at the same remote path — which `deploy/backups.md` itself half-invites by suggesting turning `BACKUP_ENABLED=true` on dev to rehearse the path — a dev run would prune production's older archives too, since `rclone delete --include '*.tar.gz.enc'` matches every stack's files, not just `ponente-dev-*`. Suggest: `--include "${STACK_NAME}-*.tar.gz.enc"` to mirror the local prune's scoping.
Prune only this stack's own archives, offsite as well as local
All checks were successful
CI / build (pull_request) Successful in 33s
64f91fc405
The remote prune matched `*.tar.gz.enc`, so a stack sharing a remote path
with another one deleted the other's archives along with its own — and
sharing a path is exactly what deploy/backups.md suggests when it offers
the dev stack as a place to rehearse the backup path.

Both prunes now go through one glob. It spells the timestamp out rather
than ending in `*` because a stack name can be a prefix of another:
`ponente-*` still matches every `ponente-dev-` archive next to it.
A string comparison at startup fails the run if the archive name and the
glob ever stop agreeing, since the symptom otherwise is a prune that
quietly matches nothing.

restore.sh keeps its unscoped `*` — it only ever lists.
Author
Collaborator

The finding was right, and it was worse than described. Fixed and pushed to claude/issue-132 (64f91fc).

The fix

The remote prune is now scoped like the local one — but not with ${STACK_NAME}-* as suggested, because that is still wrong. ponente-*.tar.gz.enc matches ponente-dev-20260821T…enc, so a production run would prune the dev stack's archives; the two-stack collision just changes direction. Both prunes now share one glob that spells the timestamp out:

ARCHIVE_GLOB="$STACK_NAME-????????T??????Z.tar.gz.enc"

? matches exactly one character and never a separator, in bash and in rclone's filters alike, so ponente- cannot reach past its own hyphen. One variable feeds rclone delete --include and the local ls, so they can't drift apart.

I verified this against rclone's actual filter engine (v1.75.0) rather than assuming the glob semantics, with all four names side by side:

--include deletes
*.tar.gz.enc (before) ponente-*, ponente-dev-*, other-*, ponente-notes — everything
ponente-*.tar.gz.enc (suggested) ponente-* plus ponente-dev-* and ponente-notes
ponente-????????T??????Z… (now) its own two, nothing else
same, as ponente-dev its own one, nothing else

One thing added

The glob is written by hand, so it can fall out of step with NAME above it — and that failure is silent in the wrong way: the prune matches nothing and the archive directory grows forever until a disk fills. A string comparison at startup, before anything is written, fails the run if tonight's archive wouldn't survive its own prune. Exercised in the real script with a shim container CLI: it passes on the real name (the run proceeds to pg_dump) and fires on a changed date format.

Not changed

restore.sh:82,88 also list with an unscoped *.tar.gz.enc. That one is correct — it only ever lists, and when you are picking an archive to restore you want to see everything in the directory, not just the stack you happen to be invoking from.

deploy/backups.md gained the reasoning in the three places it belongs: that a shared path is safe and why the pattern is spelled out, the prune step in the run description, and a line in What was rehearsed recording the table above. No changelog entry — backups are maintainer-facing.

dotnet test Ponente.sln — 380 passed, 0 failed (including DocumentationTests); bash -n clean. The full MinIO rehearsal from the original PR could not be re-run here — this container has no docker or podman — but the change is confined to the prune globs, which I tested directly with the real rclone binary.

No new issues filed.


⏱ 3m 23s · 31 turns · tokens: 39.2k in / 13.3k out (+1287.0k cache-read)
📜 Full session transcript

The finding was right, and it was worse than described. Fixed and pushed to `claude/issue-132` (`64f91fc`). ## The fix The remote prune is now scoped like the local one — but not with `${STACK_NAME}-*` as suggested, because that is still wrong. `ponente-*.tar.gz.enc` matches `ponente-dev-20260821T…enc`, so a *production* run would prune the dev stack's archives; the two-stack collision just changes direction. Both prunes now share one glob that spells the timestamp out: ```bash ARCHIVE_GLOB="$STACK_NAME-????????T??????Z.tar.gz.enc" ``` `?` matches exactly one character and never a separator, in bash and in rclone's filters alike, so `ponente-` cannot reach past its own hyphen. One variable feeds `rclone delete --include` and the local `ls`, so they can't drift apart. I verified this against rclone's actual filter engine (v1.75.0) rather than assuming the glob semantics, with all four names side by side: | `--include` | deletes | |---|---| | `*.tar.gz.enc` (before) | `ponente-*`, `ponente-dev-*`, `other-*`, `ponente-notes` — everything | | `ponente-*.tar.gz.enc` (suggested) | `ponente-*` **plus `ponente-dev-*` and `ponente-notes`** | | `ponente-????????T??????Z…` (now) | its own two, nothing else | | same, as `ponente-dev` | its own one, nothing else | ## One thing added The glob is written by hand, so it can fall out of step with `NAME` above it — and that failure is silent in the wrong way: the prune matches nothing and the archive directory grows forever until a disk fills. A string comparison at startup, before anything is written, fails the run if tonight's archive wouldn't survive its own prune. Exercised in the real script with a shim container CLI: it passes on the real name (the run proceeds to `pg_dump`) and fires on a changed date format. ## Not changed `restore.sh:82,88` also list with an unscoped `*.tar.gz.enc`. That one is correct — it only ever lists, and when you are picking an archive to restore you want to see everything in the directory, not just the stack you happen to be invoking from. `deploy/backups.md` gained the reasoning in the three places it belongs: that a shared path is safe and why the pattern is spelled out, the prune step in the run description, and a line in *What was rehearsed* recording the table above. No changelog entry — backups are maintainer-facing. `dotnet test Ponente.sln` — 380 passed, 0 failed (including `DocumentationTests`); `bash -n` clean. The full MinIO rehearsal from the original PR could not be re-run here — this container has no docker or podman — but the change is confined to the prune globs, which I tested directly with the real rclone binary. No new issues filed. --- ⏱ 3m 23s · 31 turns · tokens: 39.2k in / 13.3k out (+1287.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/issue-134/run-353.md)
Author
Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/355); the reply will link the full session transcript. <!-- claude-run:pr_review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-355.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

The remote-prune scoping bug flagged in both previous passes is now fixed: ARCHIVE_GLOB="$STACK_NAME-????????T??????Z.tar.gz.enc" is defined once and used consistently for both the local prune and the remote rclone delete --min-age ... --include "$ARCHIVE_GLOB", so a shared remote path between two stacks (e.g. rehearsing backups on dev) can no longer prune another stack's archives. There's also a nice self-consistency guard that fails loudly if the archive's own generated name ever stops matching that glob, which keeps the two in sync going forward — and the "what was rehearsed" section in deploy/backups.md explicitly documents this exact scoping scenario having been tested with sibling stack names side by side.

Everything else remains as assessed in the prior reviews: the WAL-aware SQLite .backup approach (with the measured empty-restore failure mode it avoids), the scratch-Postgres restore-and-query verification, the decrypt-and-compare round trip, deploy.sh's pre-flight guardrails (passphrase required, rclone.conf required if a remote is set, SERVER_STATE_DIR required), the backup/restore systemd units, and the very thorough deploy/backups.md runbook all look correct and well thought through. No new issues found in this pass.


⏱ 0m 26s · 3 turns · tokens: 46.4k in / 2.1k out (+181.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The remote-prune scoping bug flagged in both previous passes is now fixed: `ARCHIVE_GLOB="$STACK_NAME-????????T??????Z.tar.gz.enc"` is defined once and used consistently for *both* the local prune and the remote `rclone delete --min-age ... --include "$ARCHIVE_GLOB"`, so a shared remote path between two stacks (e.g. rehearsing backups on dev) can no longer prune another stack's archives. There's also a nice self-consistency guard that fails loudly if the archive's own generated name ever stops matching that glob, which keeps the two in sync going forward — and the "what was rehearsed" section in `deploy/backups.md` explicitly documents this exact scoping scenario having been tested with sibling stack names side by side. Everything else remains as assessed in the prior reviews: the WAL-aware SQLite `.backup` approach (with the measured empty-restore failure mode it avoids), the scratch-Postgres restore-and-query verification, the decrypt-and-compare round trip, `deploy.sh`'s pre-flight guardrails (passphrase required, rclone.conf required if a remote is set, SERVER_STATE_DIR required), the backup/restore systemd units, and the very thorough `deploy/backups.md` runbook all look correct and well thought through. No new issues found in this pass. --- ⏱ 0m 26s · 3 turns · tokens: 46.4k in / 2.1k out (+181.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-134/run-355.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:64f91fc405e22140ee487dabe14b24a164d9529a -->
Grandiras deleted branch claude/issue-132 2026-08-21 01:54:18 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/Ponente!134
No description provided.