Hash-pin every external reference in deploy.yml, verified on a real runner (closes #7) #22
Reference in New Issue
Block a user
Delete Branch "pin-deploy-refs-observable"
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?
Second attempt at #7. The first (#17) passed two adversarial reviews, merged,
and broke the deploy on
main; it was reverted as3d17e22. This one wasverified by actually running the job on a runner before merge, and doing that
found two independent breaks that neither review had caught.
Why the first attempt could not be reviewed into correctness
deploy.ymltriggers only on push tomain, so every pre-merge checksimulated the environment instead of being it.
script/cibuildstructurallycannot help:
docker buildrunsRUNunder/bin/shwith no Actions runtimeand no JS-action execution.
So this branch temporarily added itself to
on.push.branchesand added apermanent guard to the deploy job:
Branch pushes then exercise
buildfor real whiledeployreports as skipped,touching no Cloudflare project and spending no token. The Actions jobs/logs API
403s for this account, so the diagnosis was encoded as job topology instead
of log output — a temporary
probe.ymlwhose jobs each isolate one hypothesis,each surfacing as its own
contextin the commit-status API.The temporary trigger and
probe.ymlare gone in the final commit. Theif:guard is kept permanently.
Observed commit-status output
Round 1,
2d328e7— reproduce and localise:p1 vs p2:
act_runnerreally does not supplynode, so theapk addstep isrequired and sufficient. p3:
script/bootstrapandscript/testcomplete onthe pinned alpine digest. p5/p6: musl is not the problem. p2 vs p4 differ only
by a trailing
upload-artifactv4 step.Round 2,
602fd60— build goes green, a second failure appears:Round 3,
07af755— split the dry run:r2a/r2b and r3a/r3b upload and download the real site tarball across the two
job containers, so the artifact round trip is sound; wrangler is the second
break.
Round 4,
73f912c— everything green, including a full rehearsal of bothjobs:
s2-deploy-dryrunis thedeployjob as written — same pinned node digest,same pinned download action, same pinned wrangler, real site tarball
downloaded and extracted,
test -f public/index.html— withwrangler pages deploy --helpin place of the publish call.Final commit
54ed637has nobuildstatus, by design: the temporary triggeris removed there.
check / checkis green on it.Root cause 1: artifacts v4, not the image
The image swap everybody reviewed was never the problem.
actions/upload-artifactv4 is a different wire protocol from v3 and this Gitea Actions instance (server
1.25.4) does not serve it.
Deviation from the issue, stated explicitly: DoD item 2 says
upload-artifactanddownload-artifactmove v3 -> v4 because v3 isdeprecated. They do not. They stay on the v3 line, pinned by full commit SHA,
at the exact commits the mutable
@v3refs were already resolving to(
ff15f030= v3.2.1,9bc31d5c= v3.0.2) — i.e. the code that was alreadydeploying this site, now pinned instead of floating. The hash-pinning rule this
issue exists for is satisfied; the version bump is not. Evidence is p2 vs p4
above. Tracked in #20 for when the instance can serve v4.
DoD item 3 — reuse the
Dockerfile's alpine digest — is kept. The evidencesays it works fine (p3, s1), so there was no reason to deviate from it.
Root cause 2: the wrangler pin and the node pin contradict each other
wrangler@4.120.0declaresengines.node >= 22.0.0and refuses to start onthe pinned node 20 container. Reproduced locally in that exact image:
npm installonly warns aboutengines, so the install step would have passedand the deploy step would have failed — a break in the same unrunnable job.
The instructive part is what the unpinned command being replaced was doing:
npm resolves a bare name to the newest version whose
enginesthe running nodesatisfies, so
npm install -g wrangleron node 20 has been installing 4.86.0,not the 4.120.0 that
latestpoints at. Pinning "the latest version" was notpinning what we had; it was an unnoticed bump onto a node the container does
not have.
Deviation from the issue, stated explicitly: DoD item 4 asks for wrangler
pinned to an exact version, which this does — but at 4.86.0 (engines
node >= 20.3.0, published 2026-04-28), the version that has actually beendeploying this site, verified to install and run on the pinned node digest. The
node image digest is unchanged. Moving to node 22 so the wrangler pin can
advance is tracked in #21.
What is verified and what is not
Verified:
make checkgreen.script/cibuildgreen.buildjob ran green on a realact_runneron this branch (73f912c,"Successful in 7s"), with the pinned alpine digest, the prerequisite step,
script/bootstrap,script/test, the tar, and the pinned upload.tarball, using the pinned v3 pair.
(
s2-deploy-dryrun) and locally.Not verified: the
deployjob itself has never run. It needsCLOUDFLARE_API_TOKENand it publishes, so it cannot run from a branch. Everystep of it is individually rehearsed above except the final
wrangler pages deployinvocation and the Cloudflare API interaction behindit. The
mainrun must still be watched after merge and the live siteconfirmed. This PR does not claim otherwise, and the previous attempt's mistake
was exactly a claim of this kind.
Also unchanged and unclaimed: the apk packages in the prerequisite step and in
script/bootstrapresolve at run time and are not hash-pinned (repo-wide gap,tracked in #19).
Scope
Two files change against
main:.gitea/workflows/deploy.ymlandTODO.md.The two jobs stay separate, no renovate/dependabot, no
package.json. The deadfeat/initial-sitetrigger is dropped and the file is reindented to 4-spaceYAML, per DoD items 5 and 6.
Summary for review
What changed
Two files against
main,.gitea/workflows/deploy.ymlandTODO.md.The pinned values, all with a version/date comment above the reference:
buildcontaineralpine@sha256:c3f8e73f...(byte-identical toDockerfileline 6)deploycontainernode@sha256:8f693eaa...(node 20.20.2 bookworm)actions/checkout11bd71901bbe5b1630ceea73d27597364c9af683(v4.2.2, same SHAcheck.ymluses)actions/upload-artifactff15f0306b3f739f7b6fd43fb5d26cd321bd4de5(v3.2.1)actions/download-artifact9bc31d5ccc31df68ecc42ccf4149144866c47d8a(v3.0.2)wrangler@4.86.0Plus:
klakegg/hugo:ext-alpinegone; a pre-checkoutapk add --no-cache nodejs git tarstep andshell: shjob default so therunner can execute JS actions on a bare musl image;
script/bootstrapthenscript/test; the deadfeat/initial-sitetrigger dropped; 4-space YAML; andif: github.ref_name == 'main'on thedeployjob, kept permanently.Two of the six pins from the reverted attempt changed, both because the branch
run proved they do not work here. Those are called out as explicit deviations
in the PR body with the evidence, and split into #20 and #21.
How it was verified
Four rounds on the branch, each read back from the commit-status API. Full
output is in the PR body; the load-bearing lines:
The bisect was done with a temporary
probe.ymlof single-hypothesis jobs,since each job is its own status
contextand the jobs/logs API is notreadable by this account. Sixteen probe jobs across the four rounds. That file
and the temporary branch trigger are both deleted in the final commit
54ed637;check / checkis green on it.Local, in the pinned images (diagnosis, not a build path):
docker runinnode@sha256:8f693eaa...:wrangler@4.120.0installs withEBADENGINE warnings and then exits 1 with "Wrangler requires at least Node.js
v22.0.0"; unpinned
npm install -g wranglerresolves to 4.86.0; explicitwrangler@4.86.0installs and runs.Repo gates:
make checkandscript/cibuildboth green on the final tree.What a reviewer should push on
v4. My case is that v4 measurably fails here (p2 vs p4) and that the SHAs
chosen are the exact commits
@v3was already resolving to while the sitedeployed. If you think a deprecated-but-working pin is the wrong trade, that
is the argument to have.
latest. 4.86.0 is what npm was alreadyinstalling on node 20; the alternative is bumping the container to node 22
(#21), which I judged out of scope here.
submodules: recursiveand thegitpackage earn their place --there is no
.gitmodulesin the repo. I left both as-is because they camefrom the reverted work and removing them is not what this issue is about, but
it is a fair thing to flag.
What remains unproven
The
deployjob has never executed. It needsCLOUDFLARE_API_TOKENand itpublishes. Every step of it is rehearsed by
s2-deploy-dryrunexcept the finalwrangler pages deploycall and the Cloudflare API behind it. Themainrunmust be watched after merge and https://lora.vegas/ confirmed. I am not
claiming more than that -- overclaiming verification is what shipped the last
break.
Independent adversarial review — PR #22
Reviewer had no involvement in #17 or in any prior round of #7. All prior
reasoning in this thread, including the manager's, was treated as unverified.
Every claim below was re-derived from primary sources; nothing was accepted
from pasted output.
Verdict: PASS
No blocking findings. Three non-blocking observations at the end, none of
which should be acted on inside this PR.
Priority 1a — the artifact v3 deviation
Re-resolved against the GitHub API, not against the PR body.
Both SHAs are real commit objects, both genuinely correspond to the claimed v3
point releases, and — the load-bearing part — the mutable
@v3refs currentlyresolve to exactly those commits. The pins are therefore byte-for-byte the code
that was already deploying this site, now frozen instead of floating. That is
the strongest available justification for the deviation and it holds.
The deviation is disclosed honestly. The PR body carries an explicit
"Deviation from the issue, stated explicitly" heading naming DoD item 2,
states that the v3 -> v4 bump is not done, and does not dress the deprecated
line up as a virtue. It is separately tracked in #20, which exists, is open,
and states a real done-when. DoD item 2 was amended by the manager in
issue #7 comment 46903, so the deviation is sanctioned as well as disclosed.
Priority 1b — the wrangler pin
This was the subtlest claim in the PR and it was verified end to end, including
by running the pinned image directly.
From the npm registry:
Swept all 498 stable versions in the registry: 4.86.0 is the highest stable
version whose
engines.nodeadmits node 20. Every release from 4.113.0through 4.120.0 declares
>=22.0.0. So the claim that a barenpm install -g wrangleron node 20 lands on 4.86.0 is not merely plausible,it is the only version npm could pick.
Confirmed empirically in
node@sha256:8f693eaa7e0a8e71560c9a82b55fd54c2ae920a2ba5d2cde28bac7d1c01c9ba5:Every element of the PR's account reproduces: the install step passes on
4.120.0 and the failure surfaces one step later at invocation, which is exactly
the shape that would have produced a second outage after the first was fixed.
DoD item 4 asks for an exact version and gets one; 4.86.0 is additionally the
version that has actually been deploying this site.
Priority 2 — the claimed green run
Read from the commit-status API directly, not from the PR body.
73f912c7edec21a92ba298d491b872af7e48f75b—state: success, 5 statuses:54ed6376af8c5f27508f434ebd7aa7f053b7b2c6—state: success, 1 status:Both match the PR body exactly, including the absence of a
buildstatus onthe final commit (the temporary trigger is gone there, as stated).
The check that actually makes the green run mean something, and which is
not asserted anywhere in the PR body:
git diff 73f912c 54ed637touches only(a) removal of the temporary branch trigger, (b) deletion of
probe.yml,(c) three comment rewordings, and (d)
TODO.md. There is no functional changeto
deploy.ymlbetween the commit that ran green on a realact_runnerandthe commit proposed for merge. The green
buildresult therefore transfers tothe final tree. Had a single
uses:,image:, orrun:line moved after73f912c, this would have been a blocking finding.Priority 3 — final tree state
Verified against the head tree, not against intermediate commits.
.gitea/workflows/at head containscheck.ymlanddeploy.ymlonly.probe.ymlis present at73f912cand absent at head. Confirmed bygit ls-treeat both commits.onis exactly{'push': {'branches': ['main']}}— thetemporary branch trigger is gone from the parsed structure, not merely
commented out.
deploy.ifparses asgithub.ref_name == 'main'. Correct for a push tomain, and it is defence in depth given the trigger is already[main]only. Kept permanently, as requested in issue #7 comment 46551.
Dockerfileline 6:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709. Comparedcharacter by character, not by prefix.
(verified by executing the image).
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683is identical tothe SHA
.gitea/workflows/check.ymlalready pins; the two workflows agree.deploy.ymlbegins at a 2/6/10-space level. 4-spacethroughout, matching
check.ymland.editorconfig. DoD items 5 and 6 met.TODO.mdis updated in the same commit as the work: Status amended,a dated Completed entry added, the stale "Pin the images and actions" Future
Step removed and replaced by entries pointing at #20 and #21. The
no-terminal-period bullet style matches the surrounding entries.
Hash-pin every external reference in deploy.yml (closes #7). Exactly one commit in3d17e22..HEADcarries acloses #token, and it is the head commit. No intermediate commit closes the issue.
make checkgreen (prettier clean: "All matched files use Prettier codestyle!"; Hugo build succeeds). The two warnings emitted are pre-existing —
the
languageCodedeprecation and the taxonomy layout warning tracked in #13— and are not introduced by this change.
script/cibuildgreen..gitandthemes), the diff, all five commit messages and trailers, the PRbody, and the PR comment. Grep returned nothing.
package.json. No scope creep found.origin/main(3d17e22).Fast-forward, no conflicts.
with real done-when criteria rather than being placeholders.
Does the PR body overstate verification?
No. This was checked adversarially, since it is the specific repeated failure
mode in this issue's history.
The body carries a dedicated "Not verified" section stating plainly that
the
deployjob has never run, that it needsCLOUDFLARE_API_TOKENandpublishes, that every step is individually rehearsed except the final
wrangler pages deploycall and the Cloudflare API behind it, and that themainrun must still be watched after merge with the live site confirmed. Thereview comment repeats the same limitation. The unpinned apk packages are also
disclosed as an unclaimed gap with a tracking issue.
The distinction between "the
deployjob ran" and "a probe job with the sameimage, same pinned download action, same pinned wrangler and the real tarball
ran, stopping at
--help" is drawn correctly and consistently everywhere itappears. No claim in the body was found to exceed its evidence.
Non-blocking observations
None of these should be changed in this PR. The final tree is functionally
identical to a configuration proven green on a real runner, and editing it now
would forfeit that — which is the precise mistake that shipped twice already.
submodules: recursiveon the checkout step is a no-op: there is no.gitmodulesin the repo andthemesis an ordinary tree, not a gitlink.The inline comment justifies the
gitapk package by that option("git is needed for checkout's
submodules: recursive"), so it justifies adependency by a feature the repo does not use. The
gitinstall is stillindependently worth having (checkout otherwise degrades to a tarball
download). Worth a follow-up issue to drop the option and correct the
comment, verified by the same branch-trigger technique. The author flagged
this himself in the review comment, which is the right call.
Both pinned artifact actions declare
runs.using: 'node16'in theiraction.yml, whileactions/checkoutv4.2.2 declaresnode20. The currentact_runnerevidently executes them anyway (proven bybuildands2-deploy-dryrungoing green), and this is the status quo rather than aregression, but it is a durability risk: a future runner that drops node16
support breaks this deploy. Worth adding to #20 as a second reason the v3
line is a dead end.
tarin the apk prerequisite step is arguably redundant — busybox providesa
tarsufficient for thetar -czf site.tar.gz publicstep. Harmless, andpart of the proven-green configuration. Not worth touching.
Coverage note
Not performed: reading the Actions job logs (the jobs/logs API 403s for this
account, as documented). Independent status verification was limited to the two
commits under review,
73f912cand54ed637; the intermediate probe rounds(
2d328e7,602fd60,07af755) were not re-read from the API, since theirconclusions are all superseded by the two facts that were verified directly —
the final
buildresult and the empty functional diff between the tested andfinal commits. The
deployjob itself cannot be exercised pre-merge and wasnot; that limitation is the PR's own stated position and remains true.
Post-merge action, unchanged from what the PR itself asks for: watch the
mainrun and confirm https://lora.vegas/ serves a fresh deploy, with an immediate
revert if the
deployjob fails.Manager note — PASS accepted. Relabelled
merge-readyand merging now.mainis unprotected in this repo, so this merges directly rather than beingassigned to @sneak.
The decisive check in this review was one the reviewer added on its own
initiative and which nobody had asked for:
git diff 73f912c 54ed637touchesonly the temporary-trigger removal, the
probe.ymldeletion, three commentrewordings, and
TODO.md— zero functional change todeploy.ymlbetweenthe commit that was verified green on a real runner and the commit being
merged. That is the check that makes a green branch build transferable to
main. Without it, "the build passed on the branch" and "the build will passon
main" are different claims, and the gap between them is exactly where thisissue has already put the site's deploy on the floor once. Good catch.
Also accepted: both artifact SHAs re-resolved independently, and confirmed to
be the same commits the moving
@v3tags point at today — so the pins arebyte-for-byte the code that was already deploying the site, which is the
strongest possible position for a pinning change. And the wrangler claim was
verified by sweeping all 498 stable versions and by executing both versions in
the pinned image, rather than by reading the manifest and reasoning.
The three non-blocking observations are deliberately not being fixed here:
submodules: recursiveis a no-op (no.gitmodules;themesis a plaintree, not a gitlink), which also undercuts the stated justification for the
gitpackage.runs.using: 'node16', which works on thecurrent runner but is a durability risk.
apk add taris probably redundant against busybox tar.Changing the final tree now would forfeit the runner-verified green and put us
back to reasoning instead of observing — the precise mistake that shipped
twice. (1) and (3) will be filed as a follow-up; (2) is being added to #20.
Remaining and unavoidable: the
deployjob has still never executed. It needsCLOUDFLARE_API_TOKENand publishes, so it cannot be exercised from a branchunder any arrangement. I am merging and watching the
mainrun immediately,with an immediate revert if
deployfails — same as last time, except thatthis time the
buildjob it depends on has been proven on a real runner.