Repository navigation
Deterministic deploy script, rollback, and CI for implish.org - #7
tangentstorm wants to merge 6 commits into
Conversation
bin/deploy builds web/ with vite, backs up the live docroot to a timestamped copy (keeping the last DEPLOY_KEEP), and rsyncs dist-web/ over it. Supports --dry-run, --rollback [NAME] and --list-backups. Host/user/docroot come from deploy/deploy.conf or DEPLOY_* env vars. .github/workflows/deploy.yml tests on PRs and deploys on push to main or workflow_dispatch (optional ref), with a DEPLOY_SSH_KEY secret and pinned host keys in deploy/known_hosts. test/deploy.test.sh covers dry run, failed build, rollback, retention and path guards, locally and through a fake ssh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Deploy job runs in the `production` environment (deployment branches limited to main) and only on refs/heads/main. workflow_dispatch `ref` must already be merged into main (deploy/select-ref.sh). - Move the server-side half to deploy/remote.sh. Every rm -rf goes through a strict backup-name check. A failed backup copy removes its .tmp and aborts before rsync, and stale .tmp dirs are cleaned. - Hold the backup-dir flock for a whole deploy or rollback through a held ssh session, instead of locking per step. - Tests: fix the `VAR=x out=$(cmd)` idiom (VAR was never exported to cmd), report mode-specific skips, and add cases for all of the above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6297d892a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2s on #7: - Never edit the live tree in place. Each deploy uploads a complete tree into DEPLOY_RELEASES/<stamp>.tmp (unchanged files hard-linked from the live release), renames it, then repoints the DEPLOY_PATH symlink with a single rename(2). Rollback is the same atomic switch to an older release, and older releases are the backups. A failed or killed upload leaves the live site untouched; the first deploy migrates a plain docroot into the oldest release. - Keep deploy tooling separate from the source being built. CI runs bin/deploy, deploy/* and the tests from github.sha, and builds the selected merged commit from a git-archive export (DEPLOY_SOURCE_DIR), so commits from before bin/deploy existed can be redeployed. - Everything created on the target uses umask 022, so nginx can read it even when the deploy shell has umask 077 (found in a local e2e run). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a374a0b894
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 on #7 (remote.sh:64): a browser that loaded index.html from release N and fetches its hashed asset after the switch to N+1 got a 404, because URLs resolve through the docroot symlink and N+1 doesn't ship that file. Before every switch (deploy, migration of a plain docroot, rollback, roll-forward), the target release now gets hard links to every file the last DEPLOY_ASSET_GRACE (default 3) live releases shipped and it lacks. Its own files are never overwritten. Shipped files are recorded per release (RELEASES/<name>.files) and live history in RELEASES/.history, so the window is bounded and follows what browsers actually saw. Prune keeps releases in the window, and manifest entries with .. or a leading / are ignored. Tests reproduce the old-index-then-switch 404 over HTTP for deploy, rollback and roll-forward, and cover removed data files, no-overwrite, the window bound, prune vs. the window, bad manifest entries, and grace 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a357e30dc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex P2 on #7 (remote.sh:139): the first deploy onto a plain docroot moved the live directory away before the fallible carry/remember steps, so a failure (e.g. old downloads/old.json vs. a new regular file named downloads) left the site down. - Split every switch (deploy, migration, rollback, roll-forward) into `stage` (all fallible work, live site untouched) and `commit` (one rename). Post-switch history writes only warn; a prune failure says the new release is live. - Migration stages from the plain docroot before moving it, and restores it if the final rename fails. - Carry never fails a switch: conflicts (file vs. dir either way) and symlinked parents on either side are skipped with a warning. - An exit trap removes the staged symlink and any release that never went live, so retries don't collide and rollback never picks one. - DEPLOY_FAULT=<point> injects failures; tests cover each point for each switch kind, plus the conflict cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Fixes #6
What
bin/deploy: builds (npx vite build), uploads the result as a new release (<docroot>.releases/<UTC timestamp>), then atomically switches the docroot symlink to it with a singlerename(2). The live tree is never edited in place, and older releases are the backups (the newestDEPLOY_KEEP, default 5, are kept). Before every switch, files that the lastDEPLOY_ASSET_GRACE(default 3) live releases shipped are hard-linked into the target, so a page loaded just before a switch can still fetch its hashed assets. Any failure exits nonzero. If the build fails, it stops before anything on the server is touched.--dry-runbuilds, lists the itemized changes against the live tree, and touches nothing.--rollback [NAME]is an atomic switch to the release before the live one (repeat to go further back), or to NAME, which can also roll forward.--rollback --dry-runpreviews.--list-releaseslists releases and marks the live one.deploy/remote.sh: the server-side half (prepare, activate, switch, prune, pick, diff, discard), piped tobashover ssh..github/workflows/deploy.yml: PRs get build + deploy tests only. Push to main andworkflow_dispatchon main run the tests, thenbin/deployin theproductionenvironment, then a curl check of https://implish.org/.refinput, which must already be merged into main.deploy/select-ref.shchecks that and exports it withgit archive.deploy/deploy.confholds the config.deploy/known_hostspins the server's host keys, so CI never trusts a key on first use.deploy/README.mdis the full runbook. CLAUDE.md is updated to match.This box can't reach the server over ssh:
Permission denied (publickey,password). There's no private key here, and I didn't try any way around that.DEPLOY_HOST=45.79.174.182is set.DEPLOY_USERandDEPLOY_PATHare empty and have to be filled in. Until they and the secret are set,bin/deployrefuses to run.What Michal needs to add (details in deploy/README.md)
productionmainproductionenv secret (not a repo secret)DEPLOY_SSH_KEYssh-keygen -t ed25519 -N '' -C implish-deploy@github-actions -f implish-deploy)productionenv variableDEPLOY_USERproductionenv variableDEPLOY_PATHrootfor implish.org, e.g./var/www/implish/site(see the server prerequisites below)~DEPLOY_USER/.ssh/authorized_keysrestrict ssh-ed25519 AAAA…(implish-deploy.pub)… implish-deploy@github-actionsCommands:
gh secret set DEPLOY_SSH_KEY --repo tangentcode/implish --env production < implish-deployandgh variable set DEPLOY_USER|DEPLOY_PATH --env production --body …. Don't store the key as a repo-level secret: same-repo PR branches can read those, so a PR that edits the workflow could deploy. Also recommended: branch protection onmain, since everything that lands there deploys.Server prerequisites. These need root, so they're flagged to you:
/var/www, the least-privilege option is to move the site one level down into a dir the deploy user owns, and point nginx (and certbot's webroot, if it uses one) at it. For examplesudo mkdir /var/www/implish && sudo mv /var/www/implish.org /var/www/implish/site && sudo chown -R USER: /var/www/implish, setroot /var/www/implish/site;, then reload nginx.disable_symlinksisn't on./etc/ssh/ssh_host_*_key.pub.Until the secret is set, the deploy job on main fails with an error that points at deploy/README.md. I chose that over skipping so that a misconfiguration is visible.
Behavior notes
DEPLOY_ASSET_GRACEmore deploys, which is the point of the grace period. Dry runs say so. Set it to 0 to disable./.well-known/(certbot) is copied from the live tree into each new release and protected from--delete.umask 022, so nginx can read it whatever umask the deploy shell has.--link-dest), so they aren't re-sent and the releases share disk space.--checksummeans CI checkouts with fresh mtimes only transfer changed content.actions/checkout@v5/setup-node@v5(Node 24 runtime)./usr/local/bin/node20.11 can't build the site, and/usr/bin/node21.7 can.package-lock.jsonis gitignored in this repo. CI usesnpm install. Committing a lockfile would make builds fully reproducible; I left that as your call.Findings fixed while building (root cause → neighboring cases covered)
--chmodis ignored without-p. Uploads use-rlp. Test:t_deploy_migrates_plain_docrootchecks the mode is 644. Rollback no longer copies files, so the neighbor case from round 0 is gone.vars.Xto""). EveryDEPLOY_*override applies only when non-empty. Test:t_conf_file_and_env_override.--deleteorrm -rfaimed at a wrong path.DEPLOY_PATHandDEPLOY_RELEASESare validated: relative paths,/, single-level paths,.., spaces/quotes, and a releases dir inside the docroot are all refused. A missingDEPLOY_SOURCE_DIRis refused too. Test:t_rejects_unsafe_paths.Review fixes (MinBot, round 1)
The secret was reachable from unmerged code. The key and vars moved to the
productionenvironment, limited to main. Neighbors:if:;if: github.ref == 'refs/heads/main');ref(select-ref.shrequiresmerge-base --is-ancestor);Test:
t_select_ref_only_merged, which also checks that nothing is exported for a rejected ref.Every
rm -rfgoes through one name check (rm_release,^20[0-9]{6}T[0-9]{6}Z$). It coversdiscard,pruneand stale-.tmpcleanup.switch,activateanddiffvalidate names the same way. Listing uses the exact regex. Test:t_remote_rejects_bad_names.Partial uploads/backups left stray
.tmpdirs. A failed upload is discarded, and stale.tmpdirs are cleaned before each prepare and on prune. Tests:t_failed_upload_leaves_live_intact,t_killed_deploy_recovers,t_stale_tmp_removed.One lock covers the whole deploy or rollback. It's held through a coprocess ssh session and released on exit, crash or disconnect. Tests:
t_lock_blocks_concurrent_runs,t_lock_held_during_upload.t_killed_deploy_recoversalso shows that akill -9'd deploy releases the lock.Test-harness
VAR=x out=$(cmd)bug (VARwas never exported): all uses are rewritten asout=$(VAR=x cmd).Review fixes (Codex, round 2)
P2: deploy tooling was taken from the historical ref (
deploy.ymlline 29). Root cause: one checkout served as both the tooling and the source to build. Fix: both jobs check outgithub.sha(the workflow's own commit) forbin/deploy,deploy/*,package.jsonscripts and tests.select-ref.sh REF BASE DESTexports only the selected merged commit's tree viagit archive(plus.deploy-sha), andbin/deploybuilds it via the newDEPLOY_SOURCE_DIR. Neighboring cases of "tooling read from the target ref":npm run test:deployfrom the old ref'spackage.json. It now always runs fromgithub.sha.deploy/known_hostsandselect-ref.shitself come from the tooling checkout.npm installruns in the exported source dir, so the old commit gets its own dependencies..deploy-sha), not the tooling checkout's HEAD.Test:
t_redeploy_pre_tooling_commitexports the real pre-tooling commit6813323from this repo (nobin/deploy, notest:deploy), deploys it with the currentbin/deploy, and checks that the liveindex.htmlequalsgit show 6813323:web/index.html. CI usesfetch-depth: 0so the commit is available, and the test ran in CI.P2: assets were deleted before the new site was published (
bin/deployline 108). Root cause: rsync edited the live tree in place (--delay-updatesdoesn't defer deletions). Fix: the release dirs plus atomic symlink switch described above, so nothing under the live tree is ever written. Neighboring cases:.tmpand leaves live untouched;kill -9) leaves live untouched and the lock released, and the next deploy cleans the stale.tmp;switchrefuses to replace a real directory;Tests:
t_live_untouched_during_upload: an rsync wrapper checks the live tree is byte-identical before and after the transfer. The fixtures use Vite-style hashed assets, andpage_okchecks that the asset the liveindex.htmlnames actually exists.t_failed_upload_leaves_live_intact,t_killed_deploy_recoverst_first_deploy_missing_docroot,t_first_deploy_empty_docroot,t_deploy_migrates_plain_docroott_rollback_is_one_atomic_switch,t_rollback_restores_previous(byte-for-byte, then roll forward by name),t_rollback_steps_backt_prune_never_removes_live,t_switch_refuses_plain_dir,t_redeploy_reuses_unchanged_filesFound during a local end-to-end run: new dirs depended on the deploy shell's umask. With umask 077, the releases dir came out
drwx------, which nginx can't read, so every request would get a 403. Root fix:umask 022for everything that runs on the target (remote.sh and the lock session), plus an explicitchmod 755on the releases dir, since the lock session can create it first. Neighbors: the releases dir, each release dir,.well-knowncopies, and the docroot's parent dirs on a first deploy. Test:t_nginx_can_read_everythingdeploys twice underumask 077onto a missing parent path and checks every dir is o+rx and every file o+r.Review fixes (Codex, round 3)
P2: a page loaded from the old release got a 404 for its hashed JS after the switch (
remote.sh:64, thread). Root cause: URLs resolve through the docroot symlink, so a file is reachable only if the current release contains it. Keeping old releases on disk doesn't help. Fix: before every switch, the target gets hard links to every file that the lastDEPLOY_ASSET_GRACElive releases shipped and the target lacks. The switch is still one atomic rename, done after the target is complete. Neighboring cases of "a page from release N requests a file missing from release N+1", each with a test:python3 -m http.serverresolves the symlink per request, like nginx). GET/, deploy, then GET the asset that index named, across 3 consecutive deploys. →t_old_index_then_switch_no_404t_deploy_migrates_plain_docroot.t_old_index_then_rollback_no_404b4-image.json): any path is carried, not justassets/. A removed file stays reachable, and a file the new release ships itself is never overwritten by an older copy. →t_removed_data_file_stays_reachableDEPLOY_KEEP=1, and carried files are hard links, so they survive their source release being pruned. →t_prune_keeps_grace_windowRELEASES/<name>.files) are carried, not ones it carried itself, so a file ages out after N switches. →t_grace_window_is_boundedRELEASES/.history(which release was live when), not name order. This is exercised by (c) and (e), and the "ignore history" mutation fails 8 runs...or a leading/are skipped, even when they'd resolve to a real file. →t_carry_ignores_bad_manifest_entriest_grace_zero_disables_carryUpdated contracts: rollback/atomic-switch tests now assert that a release's own files (its manifest) are byte-identical and that only carried files are added.
t_rollback_restores_previouschecks that every file of the old tree is restored byte-for-byte and the only extra is the rolled-back release's asset.Review fixes (Codex, round 4)
remote.sh:139, thread). Root cause: fallible steps ran while the live site was gone or half-switched. Fix: every switch isstage(all fallible work, live site untouched) +commit(one rename) + warn-only bookkeeping. An exit trap removes the staged symlink and any never-live release on failure. Neighboring cases, each with a test:stage,before-commit,migrate-before-moveandmigrate-between-renames. →t_fault_migration_keeps_old_site; empty docroot →t_fault_empty_docroot_migrationstageandbefore-commit→t_fault_deploy_keeps_old_sitet_fault_rollback_keeps_sitet_after_switch_failures_report_livedownloads/old.jsonvs. adownloadsfile during migration →t_migration_file_vs_dir_conflictt_deploy_file_vs_dir_conflictt_deploy_dir_vs_file_conflictt_rollback_file_vs_dir_conflictt_carry_never_writes_through_symlinksMutation checks (each restored afterwards):
DEPLOY_SOURCE_DIR→ 25/53.tmpcleanup → 49/53rm_releasename check, no lockTests
test/deploy.test.shhas 46 cases. 43 run in both modes and 3 are mode-specific, for 89 runs. The two modes:DEPLOY_HOST=local, and through a fakesshthat runs the remote half locally, so the real ssh +rsync -epath is exercised. The cases cover:.tmp, umask/nginx readability;6813323.Manual check: a real
vite buildwith a local temp docroot. The dry run listed the hashed assets. The deploy migrated the plain docroot into a release and switched the symlink, and--list-releasesshowed both releases. Rollback switched back to the original tree. There have been no real deploys and nothing was written to the server.Results at head
a01ddb4dedb1b4c14f27398c86b1dbe04d22676b(Node 21.7.3 locally):npm run build: passed (1/1)npm run test:deploy: 89/89 passed (3 mode-specific skips), locally and in CI run 40984171391. The deploy job was skipped on the PR, as designed.🤖 Generated with Claude Code