restore.sh opens an archive on a bare machine; release.sh stops handing deploy.sh a host #230
No reviewers
Labels
No labels
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Grandiras/Ponente!230
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/restore-bare-machine-release-hint"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two pre-existing bugs in the deploy scripts. The diff is two lines of behaviour.
restore.shexited 1 silently on a bare machineenvval() { [[ -f "$ENV_FILE" ]] && { … }; }returns 1 whendeploy/backup/.envis missing, soBACKUP_DIR="$(envval BACKUP_DIR)"trippedset -euo pipefailand 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_DIRfalls back to./backupsBACKUP_REMOTEis empty, so--listprints "No BACKUP_REMOTE configured." and--fetchfails with that messageBACKUP_PASSPHRASEfalls through to--passphrase-file/ env / the prompt, as beforeAPP_PUBLIC_URLis empty, so a--stackrestore 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:
--listexits 0 and lists nothing, a missing or junk archive prints its error, and--fetchreports no remote. With a.envpresent, values still load (BACKUP_DIRread back).bash -nis clean on both files.release.shtold you to pass a host todeploy.shIt ended with
./deploy/deploy.sh user@server, butdeploy.shtreats its positional argument as an image tag. Following that advice would pin production to a tag nameduser@serverand stand the update timer down. Asdeploy/README.mdsays, a release is a tag, there is no deploy step:release.shhas already moved:latest, and the host follows it. The hint now says only that, plus a plain./deploy/deploy.shfor the case where the stack is pinned and needs re-arming.Not in this PR
touch .envworkaround indeploy/backups.mdexists only ondebloat(#229), not onmain, so there is nothing to remove here. Once this merges, #229 should drop that line.release.sh's header comment has the samedeploy.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
🤖 Reviewing this PR — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
33740bf86ce8f7f31627✅ Verdict: approve — no blocking issues found.
Both fixes are correct and verified against the surrounding code.
restore.sh: the oldenvvalreturned 1 with no.env, and the first caller (BACKUP_DIR="$(envval …)") inherits that status, soset -ekilled 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 (--listexits 0,--fetchreports no remote, passphrase falls through to--passphrase-file/env/prompt, and a--stackrestore now reaches its final message instead of dying on theAPP_PUBLIC_URLline after the restore completed). With.envpresent the function is unchanged. On the server.envalways 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 beforecompose pullruns, following the old advice would stand the update timer down on a bogus tag. The new text matchesdeploy.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.comheader lines, still carries the old trailing echo and oldenvval(so a three-way merge keeps this PR's fixes in either merge order), and main'sbackups.mdindeed has notouch .envline 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 .envworkaround from itsbackups.mdcopy 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