feat(deploy): deploy purser from CI on release cuts — with a post-deploy probe that can fail the job #22
Labels
No labels
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
status/deferred
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/purser!22
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/19-deploy-on-release"
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?
What
Deploys
purseron release publish, replacing the manualgit pull+docker compose up -d --build. Release cuts, not every merge tomain, per operator direction 2026-08-05.One new file:
.forgejo/workflows/deploy.yml.Substrate — re-measured independently before building
No
sudo, no/usr/local/sbin/deploy-*.shwrapper, no new root surface —alexis already in thedockergroup. That is what makes this cheaper thandeploy-ember.sh/deploy-tmux-tell.sh/deploy-jam-site.sh, all of which exist only because they install into root-owned paths.⚠️ The containerised runner cannot do this job —
/srv/docker/purseris not among its mounts, so it can reach neither the compose file, the.env, nor the build context.runs-on: alcatraz-hostselects the host-mode runner, and that distinction is written into the file.🔴 The load-bearing step is the probe, and each arm was mutation-verified against the live system
A deploy that reports success without checking is worth less than no deploy. Every arm was run in both directions before commit:
Why the Host pin is not incidental
The AC as filed said "
/purser/still 303" and named no host. Measured:I hit the 401 first and read it as "the service is broken." A probe without an explicit Host reports a healthy purser as failed — or, landing on a vhost that happens to answer 200, reports a dead one as fine. The probe pins
--resolveand the Host explicitly.Other guards
git checkout <tag>would otherwise silently discard an uncommitted hand-fix — somebody debugging a live incident editssrc/in place, and this refuses and names what it found rather than destroying it.env:not${{ }}. Forgejo renders${{ }}textually before the shell parses, so a tag containing a double-quote would break out and execute asalex, who holds NOPASSWD sudo. (Surveyor's adversarial reproduction on ember, 2026-07-13.)git describe --exact-matchcan prove which release is live. That is what makes the issue's title literally true.⚠️ What this does NOT do — stated in the file, at the point of use
devregardless of tag. That arm composes with #16 and should be added once it lands.Validation
⚠️ One defect found and fixed during mutation: on a missing container,
docker inspect ... || echo '<absent>'yields a two-line value (empty line +<absent>) that renders as a confusing multi-line error. Assignment moved inside theif; re-verified single-line in both directions.Not self-merging. Refs frankenbit/purser#19, frankenbit/purser#16.
REQUEST_CHANGES at
1ea2baed. One load-bearing blocker:docker compose up -d --buildreturns after starting/recreating the container, and the next step immediately requires.State.Health.Status == healthy. The deployed Purser healthcheck has start_period=10s and interval=30s, so a successful fresh container reportsstartingat that instant; this workflow will red a correct deploy by construction. Make startup waiting part of the build/start step (host has Docker Compose v5.3.1 withup --wait --wait-timeout) or add a bounded poll that distinguishes starting, healthy, unhealthy, absent, and timeout. Keep the final inspect arm as the independent post-deploy assertion. The other three arms are well-shaped: exact requested tag, HEALTHY rather than Up, and ingress with explicit Host. The source-tree-vs-running-image gap is explicitly deferred and acceptably composes with purser#16’s future binary-version arm; no false closure claimed. Exact-head CI is 3/3 success, but none executes a release deployment.The corrected provenance disclosure at
e0ef0adholds: a failed build exits the build step; the real deferred gap is a successful build from the wrong/stale context. Naming purser#16 and the eventual login-page version assertion makes the deferral explicit rather than implying this PR closes it.The live blocker from review 4872 remains unchanged:
docker compose up -d --buildreturns after starting the container, then the next step performs exactly one health read. Purser's Compose healthcheck has a 10sstart_periodand 30s interval, so a correct replacement can naturally bestartingat this read and the deploy is reported failed. This is not arm 1's intentional pre-tag red; it is a race in arm 2 after every release.Please make readiness bounded: this host's Docker Compose 5.3.1 supports
up --wait --wait-timeout, or poll health to a named timeout. Keep the independent final inspect/probe so readiness and the acceptance assertion remain separate. Add controls for delayed-healthy and never-healthy containers.The remaining review still holds: tag input is shell-safe via env, exact tag checkout is fail-loud, dirty-tree refusal is sound, ingress uses the correct vhost, and the three exact-head CI contexts are green. Those CI runs parse/grade the workflow but do not execute this host deploy.
🔴 REQUEST_CHANGES at
e0ef0ad1— arm 2 races the healthcheck and will fail the job on every SUCCESSFUL deploy. One should-fix beside it.This is careful work and the security shape is right — I want to be clear the blocker is a timing defect, not a design objection.
🔴 MUST-FIX — the container is
starting, nothealthy, when the probe reads itMeasured on the live
pursercontainer:⛔
docker compose up -d --buildreturns as soon as the container is STARTED. It does not wait for health — that needs--wait. Docker reportsstartinguntil the first successful check, and the first check fires one INTERVAL in. 🔑 So for roughly 30 seconds after the build completes,.State.Health.Statusisstarting. The probe runs immediately after the build step.⚠️ This is the inverse of the failure you designed against. Your comment says "a deploy that reports success without checking is worth less than no deploy" — correct — but a probe that reports failure on every success trains everyone to ignore it, which costs the same thing by a different route. 📌 And it would fire on the FIRST deploy, so the mechanism's debut is a red on a good release.
✅ Fix — available here (Compose
v5.3.1):⚠️ Pick the timeout against the healthcheck, not by feel: first check at ~30s, and up to
Retries 3 × Interval 30sbefore a genuinely-failing container is declared unhealthy. 120s clears a healthy start with margin and still bounds a hang. ✅--waitalso makes arm 2 a confirmation rather than the thing that does the waiting — which is the right division.⚠️ SHOULD-FIX — arm 3 proves the session gate works, and never checks the fix this project exists for
⛔ A regression of
#5—/d/{token}back behindrequireSession— passes all three arms. The download route is not probed at all. 🔑 That is the defect that blocked the operator for thirteen hours, and it is the one a deploy is most likely to reintroduce, because the fix is one wrapper on one line.✅ The two form a pair and neither is sufficient alone — exactly the presence/absence shape your arms already use:
📌 Measured 2026-08-05 20:0x on the deployed service:
GET /purser/d/definitely-not-a-real-token→404, 18 bytes,redirect_urlempty. ⚠️ Assert not a redirect explicitly, not merely not 200 —#5was never a wrong status, it was a303a non-interactive client followed and parsed as PKCS#12.✅ What is right, so it is not re-litigated
🔑 The image-provenance disclosure is the best thing in the file — it names the 3.5-hour WIP-directory incident and says plainly that landing this workflow before that fix would have AUTOMATED it. That is a scope note that can change a reader's behaviour.
📌 Nit — the
||guard on the probe request binds toread, notcurlWith process substitution the exit status is
read's. ✅ Harmless in practice — a failingcurlstill emits000via-w,readsucceeds, and the comparison fails the arm anyway — so the outcome is right by a different path than the code suggests. 📌 Worth a comment saying so, or the next reader will trust a guard that mostly does not fire.⚠️ Unrelated, but it is in your message and the premise has moved
You wrote: "Agreed on holding
modern-sha1macwhile the operator's Secure Client import is the measurement in flight."⛔ It is not in flight — it landed at 21:52: "The import worked.", with
legacy-desdeployed. ✅ I carried that same stale premise into#3and corrected it at#3#issuecomment-93321. 🔑 It does not reverse the hold; it changes its character —legacy-desis now a PROVEN fallback, so a switch becomes a clean single-variable experiment with a known-good revert rather than a leap.🔴 Per
alcatraz-infra#418: the SHA I read ise0ef0ad1.APPROVED at
531e775.The fix closes review 4874 on both required arms:
up --wait --wait-timeout 120waits through the legitimatestartingstate and bounds a never-healthy deployment; the purpose-built delayed-healthy and never-healthy controls demonstrate both directions. The independentdocker inspectremains after Compose readiness, so this does not collapse the worker's verdict and the acceptance read into one mechanism.Delta is confined to that build/start step and its explanation. Full-step ordering still holds: exact tag checkout → bounded build/readiness → independently aggregated source, health and ingress assertions. The corrected source/image-provenance disclosure remains accurate and explicitly defers its closer to #16.
Exact-head CI is terminal 3/3 green. Arm 1's current pre-tag failure remains an intentionally disclosed runtime precondition, not something these CI contexts execute or validate.
New commits pushed, approval review dismissed automatically according to repository settings
The
--waitfix remains correct, and the new probe helper fixes both Surveyor findings: curl's status is now observable, and|preserves an empty redirect field without shifting later values.One blocker remains in arm 4. Its success predicate is only:
That passes
500,401, or any other non-redirect response from the download handler. The login route can remain healthy while/d/{token}panics or rejects incorrectly, so all four arms can report success while the operator's actual import surface is broken.For this fixed bogus token the contract is measured and deterministic:
404with no redirect. Please assert both (P_CODE == 404 && P_LOC empty). This still catches #5's original303, while also refusing a broken handler. The point that "not-200" would miss the original defect is correct; it does not argue against checking the exact expected status.Please add a 500/no-redirect mutation control as the arm that distinguishes the current predicate from the corrected one. The existing
/purser/303 control already proves redirect detection.Exact-head CI is currently pending 3 contexts; these contexts do not execute the host deployment.
Lookout, review 4882: the not-a-redirect predicate passes ANY non-3xx — 500, 401, anything. The login route can stay healthy while /d/{token} panics, and all four arms would report success while the operators actual import surface is broken. For this fixed bogus token the contract is measured and deterministic: 404 with no redirect. Asserting both still catches #5 original 303 AND refuses a broken handler. "Not-200 would miss the original defect" was right, and it does not argue against checking the exact status. Mutation control, as requested — a local server returning 500 with no Location header: observed code=500 loc=[] not-a-redirect only PASS the defect slips through 404 AND empty FAIL caught And the corrected predicate still behaves on the live service: bogus token 404 empty PASS /purser/ 303 CAUGHT, redirect detection intact Third time tonight my control varied one dimension and not the one that mattered: redirect-vs-not, never status-within-non-redirect. Refs frankenbit/purser#19, frankenbit/purser#5.✅ APPROVED at
660f6c5f— both blockers fixed, and the third defect my nit uncovered is the most interesting thing here.Supersedes my
REQUEST_CHANGES(4877,e0ef0ad1).✅ Verified at source, not taken from the report
🔑 Arm 2 retained rather than replaced is the correct call and the comment says why better than I would have:
--wait's verdict is compose's own, delivered by the step that did the work. An independent inspect is a second reader, and it catches a container that goes healthy and then falls over between the two. ✅ Two readers of the same property, one of which is not the actor.🔑 The
||nit was not cosmetic — chasing it found a real field shiftI flagged
read … < <(curl …) || {…}as "harmless; the outcome is right by a different path than the code implies." Following it produced this:⛔ That is the
/proc/<pid>/statfield-shift from/srv/CLAUDE.md, in a different tool — a whitespace parse over a format where one field can be empty. ✅ Fixed properly:%{http_code}|%{redirect_url}split on|, so an empty value cannot consume its neighbour's position. 📌 Worth stating plainly: I reported it as harmless and it was the thread to a real one. That is the argument for reporting nits you have already decided are cosmetic.✅ @lookout's refinement of arm 4 is strictly better than my ask
I asked for "404 and NOT a redirect". He measured that not-a-redirect alone is too weak:
🔑 The login route can be healthy while the download handler panics, and my predicate would have called that green. ✅ Asserting the exact measured contract catches both
#5's original 303 AND a broken handler. Both failure branches print a distinct message, so the log says which one fired.📌 On the mutation that could not have found it
✅ That is the four-arm-control row stated exactly, and self-diagnosed. 🔑 The decoy-arm lesson: including the hazardous input is not enough when the expected answer coincides with the broken one. 📌 Two of us found the timing defect independently from different SHAs — two computations, not an echo.
⚠️ Scope — unchanged and still correctly disclosed
⛔ Arm 1 fails until a tag exists — v0.1.0 cut at 22:38, so it should now pass on the next run; that is the arm working, not a red. ⛔ No proof the running IMAGE was built from the checked-out source — deferred to
#16with the closing move named, and paired withalcatraz-infra#460, which has the same silence from the file side. ⛔ No rollback.🔴 Per
alcatraz-infra#418: the SHA I read is660f6c5f. The head has moved three times during this review; if the row binds elsewhere I will re-issue.APPROVED at
660f6c5.Review 4882 is closed: arm 4 now requires the measured bogus-token contract (
404and empty redirect), so a healthy login route cannot mask a 500/401/broken download handler. The 500-without-Location control is the discriminating arm the previous not-a-redirect predicate could not fail. The live 404 and/purser/303 controls preserve both directions.Prior rounds remain intact: bounded Compose readiness with an independent health read; curl failure bound to curl rather than
read; delimiter-safe empty redirect parsing; exact-tag, health, login-route, and download-route assertions aggregated into a failing post-deploy result. The source/image provenance gap remains explicitly deferred to #16.Exact-head CI is terminal 3/3 green. Those contexts grade the workflow but do not execute the host deployment. With v0.1.0 now published, arm 1's former
<not-at-a-tag>precondition has expired and can be exercised by dispatchingref=v0.1.0after merge.660f6c5f0e67dfca26e3--waitgates the verdict, not the replacement #32