restore.sh opens an archive on a bare machine; release.sh stops handing deploy.sh a host #230

Merged
Grandiras merged 1 commit from fix/restore-bare-machine-release-hint into main 2026-09-23 15:18:20 +00:00
Owner

Two pre-existing bugs in the deploy scripts. The diff is two lines of behaviour.

restore.sh exited 1 silently on a bare machine

envval() { [[ -f "$ENV_FILE" ]] && { … }; } returns 1 when deploy/backup/.env is missing, so BACKUP_DIR="$(envval BACKUP_DIR)" tripped set -euo pipefail and the script died with no output. That is the rebuild-from-nothing case the script exists for. Now a missing file returns 0 and reads as empty:

  • BACKUP_DIR falls back to ./backups
  • BACKUP_REMOTE is empty, so --list prints "No BACKUP_REMOTE configured." and --fetch fails with that message
  • BACKUP_PASSPHRASE falls through to --passphrase-file / env / the prompt, as before
  • APP_PUBLIC_URL is empty, so a --stack restore ends with "No APP_PUBLIC_URL here to check from outside". Before this, the restore finished and then the script died silently on that line.

Checked in a directory holding only the script: --list exits 0 and lists nothing, a missing or junk archive prints its error, and --fetch reports no remote. With a .env present, values still load (BACKUP_DIR read back). bash -n is clean on both files.

release.sh told you to pass a host to deploy.sh

It ended with ./deploy/deploy.sh user@server, but deploy.sh treats its positional argument as an image tag. Following that advice would pin production to a tag named user@server and stand the update timer down. As deploy/README.md says, a release is a tag, there is no deploy step: release.sh has already moved :latest, and the host follows it. The hint now says only that, plus a plain ./deploy/deploy.sh for the case where the stack is pinned and needs re-arming.

Not in this PR

  • The touch .env workaround in deploy/backups.md exists only on debloat (#229), not on main, so there is nothing to remove here. Once this merges, #229 should drop that line.
  • release.sh's header comment has the same deploy.sh root@… mistake. I left it alone because #229 deletes those lines. Editing them here would only create a conflict.

Maintainer-only, so no changelog entry.

🤖 Generated with Claude Code

Two pre-existing bugs in the deploy scripts. The diff is two lines of behaviour. ### `restore.sh` exited 1 silently on a bare machine `envval() { [[ -f "$ENV_FILE" ]] && { … }; }` returns 1 when `deploy/backup/.env` is missing, so `BACKUP_DIR="$(envval BACKUP_DIR)"` tripped `set -euo pipefail` and the script died with no output. That is the rebuild-from-nothing case the script exists for. Now a missing file returns 0 and reads as empty: - `BACKUP_DIR` falls back to `./backups` - `BACKUP_REMOTE` is empty, so `--list` prints "No BACKUP_REMOTE configured." and `--fetch` fails with that message - `BACKUP_PASSPHRASE` falls through to `--passphrase-file` / env / the prompt, as before - `APP_PUBLIC_URL` is empty, so a `--stack` restore ends with "No APP_PUBLIC_URL here to check from outside". Before this, the restore finished and *then* the script died silently on that line. Checked in a directory holding only the script: `--list` exits 0 and lists nothing, a missing or junk archive prints its error, and `--fetch` reports no remote. With a `.env` present, values still load (`BACKUP_DIR` read back). `bash -n` is clean on both files. ### `release.sh` told you to pass a host to `deploy.sh` It ended with `./deploy/deploy.sh user@server`, but `deploy.sh` treats its positional argument as an **image tag**. Following that advice would pin production to a tag named `user@server` and stand the update timer down. As `deploy/README.md` says, *a release is a tag, there is no deploy step*: `release.sh` has already moved `:latest`, and the host follows it. The hint now says only that, plus a plain `./deploy/deploy.sh` for the case where the stack is pinned and needs re-arming. ### Not in this PR - **The `touch .env` workaround in `deploy/backups.md`** exists only on `debloat` (#229), not on `main`, so there is nothing to remove here. Once this merges, #229 should drop that line. - **`release.sh`'s header comment** has the same `deploy.sh root@…` mistake. I left it alone because #229 deletes those lines. Editing them here would only create a conflict. Maintainer-only, so no changelog entry. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
restore.sh opens an archive on a bare machine; release.sh stops handing deploy.sh a host
All checks were successful
CI / build (pull_request) Successful in 41s
33740bf86c
restore.sh read its .env through a helper that returned 1 when the file was
missing, so on a machine with no deploy/backup/.env — the rebuild-from-nothing
case the script exists for — the first `BACKUP_DIR="$(envval ...)"` tripped
`set -e` and it exited 1 without a word. A missing .env now reads as empty:
BACKUP_DIR falls back to ./backups, BACKUP_REMOTE is unset (--list and --fetch
already say so), the passphrase falls through to the prompt, and the end of a
--stack restore says there is no APP_PUBLIC_URL to check instead of dying
after the restore succeeded.

release.sh ended with `./deploy/deploy.sh user@server`, but deploy.sh's
positional argument is an image tag — following it would have pinned
production to a tag called `user@server`. A release needs no deploy.sh at
all: production follows :latest, which release.sh has just moved. The hint
now says that, and names a plain deploy.sh only for re-arming a pinned stack.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript. <!-- opencode-run:review --> 📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-230/run-2026-09-23T15-12-19-682Z.md) — updating every ~5 minutes while the review runs.
Grandiras force-pushed fix/restore-bare-machine-release-hint from 33740bf86c
All checks were successful
CI / build (pull_request) Successful in 41s
to e8f7f31627
All checks were successful
CI / build (pull_request) Successful in 47s
2026-09-23 15:16:42 +00:00
Compare
opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Both fixes are correct and verified against the surrounding code.

restore.sh: the old envval returned 1 with no .env, and the first caller (BACKUP_DIR="$(envval …)") inherits that status, so set -e killed the script silently — precisely the bare-machine recovery case the header promises. The new guard returns 0/empty, and every consumer already handles an empty value as described (--list exits 0, --fetch reports no remote, passphrase falls through to --passphrase-file/env/prompt, and a --stack restore now reaches its final message instead of dying on the APP_PUBLIC_URL line after the restore completed). With .env present the function is unchanged. On the server .env always exists (deploy.sh syncs it), so no behavior change where backups normally run.

release.sh: the old hint was worse than cosmetic — deploy.sh's positional argument is an image tag (line 78), and since the pin is written to the server's env before compose pull runs, following the old advice would stand the update timer down on a bogus tag. The new text matches deploy.sh's own re-arm message and the README's "a release is a tag" flow.

Cross-checked #229 (debloat) as the description claims: its branch deletes the root@my-server.example.com header lines, still carries the old trailing echo and old envval (so a three-way merge keeps this PR's fixes in either merge order), and main's backups.md indeed has no touch .env line to remove here. Leaving the header comment for #229 is reasonable and acknowledged.

No blockers. One thing for the follow-up on #229: after this merges, drop the touch .env workaround from its backups.md copy as the description says — the underlying bug it papered over is gone.


⏱ 5m 22s · 9 steps · $0.0195 · tokens: 56.3k in / 2.2k out (+184.8k cache-read) · Go: 5h 1% · wk 84% · mo 74%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Both fixes are correct and verified against the surrounding code. **`restore.sh`**: the old `envval` returned 1 with no `.env`, and the first caller (`BACKUP_DIR="$(envval …)"`) inherits that status, so `set -e` killed the script silently — precisely the bare-machine recovery case the header promises. The new guard returns 0/empty, and every consumer already handles an empty value as described (`--list` exits 0, `--fetch` reports no remote, passphrase falls through to `--passphrase-file`/env/prompt, and a `--stack` restore now reaches its final message instead of dying on the `APP_PUBLIC_URL` line after the restore completed). With `.env` present the function is unchanged. On the server `.env` always exists (deploy.sh syncs it), so no behavior change where backups normally run. **`release.sh`**: the old hint was worse than cosmetic — `deploy.sh`'s positional argument is an image tag (line 78), and since the pin is written to the server's env before `compose pull` runs, following the old advice would stand the update timer down on a bogus tag. The new text matches `deploy.sh`'s own re-arm message and the README's "a release is a tag" flow. **Cross-checked #229 (debloat)** as the description claims: its branch deletes the `root@my-server.example.com` header lines, still carries the old trailing echo and old `envval` (so a three-way merge keeps this PR's fixes in either merge order), and main's `backups.md` indeed has no `touch .env` line to remove here. Leaving the header comment for #229 is reasonable and acknowledged. No blockers. One thing for the follow-up on #229: after this merges, drop the `touch .env` workaround from its `backups.md` copy as the description says — the underlying bug it papered over is gone. --- ⏱ 5m 22s · 9 steps · $0.0195 · tokens: 56.3k in / 2.2k out (+184.8k cache-read) · Go: 5h 1% · wk 84% · mo 74% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/Ponente/pr-230/run-2026-09-23T15-12-19-682Z.md) <!-- opencode-reviewed-head:33740bf86ce155d92dfc25a55323cfefa1523155 --> <!-- opencode-verdict:approve -->
Grandiras deleted branch fix/restore-bare-machine-release-hint 2026-09-23 15:18:20 +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!230
No description provided.