Second attempt at #7. The first (#17) passed two adversarial reviews, merged,
and broke the deploy on main; it was reverted as 3d17e22. This one was
verified 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.yml triggers only on push to main, so every pre-merge check
simulated the environment instead of being it. script/cibuild structurally
cannot help: docker build runs RUN under /bin/sh with no Actions runtime
and no JS-action execution.
So this branch temporarily added itself to on.push.branches and added a
permanent guard to the deploy job:
deploy:needs:buildif:github.ref_name == 'main'
Branch pushes then exercise build for real while deploy reports 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.yml whose jobs each isolate one hypothesis,
each surfacing as its own context in the commit-status API.
The temporary trigger and probe.yml are gone in the final commit. The if:
guard is kept permanently.
p1 vs p2: act_runner really does not supply node, so the apk add step is
required and sufficient. p3: script/bootstrap and script/test complete on
the pinned alpine digest. p5/p6: musl is not the problem. p2 vs p4 differ only
by a trailing upload-artifact v4 step.
Round 2, 602fd60 — build goes green, a second failure appears:
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 both
jobs:
overall: success
Build and Deploy to Cloudflare Pages / build success 7s
Build and Deploy to Cloudflare Pages / deploy skipped
check / check success 6s
probe / s1-build success 13s
probe / s2-deploy-dryrun success 10s
s2-deploy-dryrun is the deploy job as written — same pinned node digest,
same pinned download action, same pinned wrangler, real site tarball
downloaded and extracted, test -f public/index.html — with wrangler pages deploy --help in place of the publish call.
Final commit 54ed637 has no build status, by design: the temporary trigger
is removed there. check / check is green on it.
Root cause 1: artifacts v4, not the image
The image swap everybody reviewed was never the problem. actions/upload-artifact
v4 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-artifact and download-artifact move v3 -> v4 because v3 is
deprecated. They do not. They stay on the v3 line, pinned by full commit SHA,
at the exact commits the mutable @v3 refs were already resolving to
(ff15f030 = v3.2.1, 9bc31d5c = v3.0.2) — i.e. the code that was already
deploying 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 evidence
says 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.0 declares engines.node >= 22.0.0 and refuses to start on
the pinned node 20 container. Reproduced locally in that exact image:
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler@4.120.0; wrangler --version'
install exit=0 # npm only warns: EBADENGINE
Wrangler requires at least Node.js v22.0.0. You are using v20.20.2.
version exit=1
npm install only warns about engines, so the install step would have passed
and 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:
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler; wrangler --version'
`-- wrangler@4.86.0
4.86.0
npm resolves a bare name to the newest version whose engines the running node
satisfies, so npm install -g wrangler on node 20 has been installing 4.86.0,
not the 4.120.0 that latest points at. Pinning "the latest version" was not
pinning 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 been
deploying 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 check green.
script/cibuild green.
The build job ran green on a real act_runner on this branch (73f912c,
"Successful in 7s"), with the pinned alpine digest, the prerequisite step, script/bootstrap, script/test, the tar, and the pinned upload.
The artifact round trip across both job containers, with the real site
tarball, using the pinned v3 pair.
wrangler 4.86.0 installing and running in the pinned node digest, both in CI
(s2-deploy-dryrun) and locally.
Not verified: the deploy job itself has never run. It needs CLOUDFLARE_API_TOKEN and it publishes, so it cannot run from a branch. Every
step of it is individually rehearsed above except the final wrangler pages deploy invocation and the Cloudflare API interaction behind
it. The main run must still be watched after merge and the live site
confirmed. 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/bootstrap resolve at run time and are not hash-pinned (repo-wide gap,
tracked in #19).
Scope
Two files change against main: .gitea/workflows/deploy.yml and TODO.md.
The two jobs stay separate, no renovate/dependabot, no package.json. The dead feat/initial-site trigger is dropped and the file is reindented to 4-space
YAML, per DoD items 5 and 6.
Second attempt at #7. The first (#17) passed two adversarial reviews, merged,
and broke the deploy on `main`; it was reverted as `3d17e22`. This one was
verified 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.yml` triggers only on push to `main`, so every pre-merge check
simulated the environment instead of being it. `script/cibuild` structurally
cannot help: `docker build` runs `RUN` under `/bin/sh` with no Actions runtime
and no JS-action execution.
So this branch temporarily added itself to `on.push.branches` and added a
permanent guard to the deploy job:
```yaml
deploy:
needs: build
if: github.ref_name == 'main'
```
Branch pushes then exercise `build` for real while `deploy` reports 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.yml` whose jobs each isolate one hypothesis,
each surfacing as its own `context` in the commit-status API.
The temporary trigger and `probe.yml` are gone in the final commit. The `if:`
guard is kept permanently.
## Observed commit-status output
Round 1, `2d328e7` — reproduce and localise:
```
check / check success 10s
Build and Deploy .../ build failure 15s reproduced the main failure
Build and Deploy .../ deploy skipped guard working
probe / p1-bare-alpine-checkout failure 3s
probe / p2-alpine-apk-checkout success 5s
probe / p3-alpine-apk-build success 15s
probe / p4-alpine-apk-upload failure 11s
probe / p5-node20alpine-checkout success 8s
probe / p6-node20slim-checkout success 11s
```
p1 vs p2: `act_runner` really does not supply `node`, so the `apk add` step is
required and sufficient. p3: `script/bootstrap` and `script/test` complete on
the pinned alpine digest. p5/p6: musl is not the problem. p2 vs p4 differ only
by a trailing `upload-artifact` v4 step.
Round 2, `602fd60` — build goes green, a second failure appears:
```
check / check success 6s
Build and Deploy .../ build success 20s
Build and Deploy .../ deploy skipped
probe / q1-upload-v3-node16 success 7s
probe / q2-upload-v3-node20 success 22s
probe / q3-build-for-roundtrip success 11s
probe / q4-deploy-dryrun failure 43s
```
Round 3, `07af755` — split the dry run:
```
check / check success 8s
Build and Deploy .../ build success 8s
Build and Deploy .../ deploy skipped
probe / r1-wrangler-only failure 7s
probe / r2a-upload-proven success 12s
probe / r2b-download-proven success 2s
probe / r3a-upload-node20 success 8s
probe / r3b-download-node20 success 2s
```
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 both
jobs:
```
overall: success
Build and Deploy to Cloudflare Pages / build success 7s
Build and Deploy to Cloudflare Pages / deploy skipped
check / check success 6s
probe / s1-build success 13s
probe / s2-deploy-dryrun success 10s
```
`s2-deploy-dryrun` is the `deploy` job as written — same pinned node digest,
same pinned download action, same pinned wrangler, real site tarball
downloaded and extracted, `test -f public/index.html` — with
`wrangler pages deploy --help` in place of the publish call.
Final commit `54ed637` has no `build` status, by design: the temporary trigger
is removed there. `check / check` is green on it.
## Root cause 1: artifacts v4, not the image
The image swap everybody reviewed was never the problem. `actions/upload-artifact`
v4 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-artifact` and `download-artifact` move v3 -> v4 because v3 is
deprecated. They do not. They stay on the v3 line, pinned by full commit SHA,
at the exact commits the mutable `@v3` refs were already resolving to
(`ff15f030` = v3.2.1, `9bc31d5c` = v3.0.2) — i.e. the code that was already
deploying 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 evidence
says 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.0` declares `engines.node >= 22.0.0` and refuses to start on
the pinned node 20 container. Reproduced locally in that exact image:
```
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler@4.120.0; wrangler --version'
install exit=0 # npm only warns: EBADENGINE
Wrangler requires at least Node.js v22.0.0. You are using v20.20.2.
version exit=1
```
`npm install` only warns about `engines`, so the install step would have passed
and 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:
```
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler; wrangler --version'
`-- wrangler@4.86.0
4.86.0
```
npm resolves a bare name to the newest version whose `engines` the running node
satisfies, so `npm install -g wrangler` on node 20 has been installing **4.86.0**,
not the 4.120.0 that `latest` points at. Pinning "the latest version" was not
pinning 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 been
deploying 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 check` green.
- `script/cibuild` green.
- The `build` job ran green on a real `act_runner` on this branch (`73f912c`,
"Successful in 7s"), with the pinned alpine digest, the prerequisite step,
`script/bootstrap`, `script/test`, the tar, and the pinned upload.
- The artifact round trip across both job containers, with the real site
tarball, using the pinned v3 pair.
- wrangler 4.86.0 installing and running in the pinned node digest, both in CI
(`s2-deploy-dryrun`) and locally.
**Not verified:** the `deploy` job itself has never run. It needs
`CLOUDFLARE_API_TOKEN` and it publishes, so it cannot run from a branch. Every
step of it is individually rehearsed above except the final
`wrangler pages deploy` invocation and the Cloudflare API interaction behind
it. The `main` run must still be watched after merge and the live site
confirmed. 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/bootstrap` resolve at run time and are not hash-pinned (repo-wide gap,
tracked in #19).
## Scope
Two files change against `main`: `.gitea/workflows/deploy.yml` and `TODO.md`.
The two jobs stay separate, no renovate/dependabot, no `package.json`. The dead
`feat/initial-site` trigger is dropped and the file is reindented to 4-space
YAML, per DoD items 5 and 6.
Restores the hash-pinning work reverted in 3d17e22 (originally 3f91a7c and
b157bfd) verbatim -- all six pinned values were independently re-resolved and
confirmed correct twice, so they are reused, not re-derived.
What is different this time is that the path is observable before it reaches
main. The previous attempt broke the deploy because deploy.yml triggers only on
push to main, so every pre-merge check simulated the runner instead of being
it, and two adversarial reviews could not catch what neither could execute.
Three changes on top of the restored work:
- A temporary development-only branch trigger on on.push.branches, so the
build job actually executes under act_runner. Removed before merge.
- if: github.ref_name == 'main' on the deploy job. Without it, a branch push
would run wrangler pages deploy against the real Cloudflare project with the
real token on every iteration. This guard is permanent: it is one line and it
makes any future branch trigger, deliberate or accidental, unable to reach
Cloudflare.
- A temporary .gitea/workflows/probe.yml, also deleted before merge. The
Actions jobs and logs API is not readable by this account; the commit-status
API is, and it reports one entry per job. So the diagnosis is encoded as job
topology rather than log output: six jobs, each isolating one hypothesis
about the 22s failure (bare alpine vs apk prerequisites, checkout vs site
build vs artifact upload, musl node vs glibc node), each surfacing as its own
status context so a single push tests them all in parallel.
make check is green. No pinned value is touched.
Round 1 of the branch probes reproduced the main failure and localised it.
Observed commit-status output for 2d328e7:
check / check success 10s
Build and Deploy .../ build failure 15s <- reproduced
Build and Deploy .../ deploy skipped <- if: guard working
probe / p1-bare-alpine-checkout failure 3s
probe / p2-alpine-apk-checkout success 5s
probe / p3-alpine-apk-build success 15s
probe / p4-alpine-apk-upload failure 11s
probe / p5-node20alpine-checkout success 8s
probe / p6-node20slim-checkout success 11s
Reading that:
- p1 vs p2: act_runner does not supply node for JavaScript actions, so the
`apk add --no-cache nodejs git tar` prerequisite step is genuinely required
and genuinely sufficient. checkout then runs on musl.
- p3: script/bootstrap and script/test complete inside the Actions container
on the pinned alpine digest. The mandated image replacement was never the
problem.
- p2 vs p4: the only difference is a trailing upload-artifact v4 step, and it
is the difference between success and failure.
- p5/p6: musl is not the issue -- checkout runs on both musl and glibc images.
So what broke the deploy was not the image swap that everyone reviewed, it was
the v3 -> v4 artifact bump that nobody questioned. Gitea 1.25.4's artifact
backend and this runner do not serve the v4 protocol; the workflow used v3
before this issue and that is what worked.
The artifact actions therefore move back to the v3 line, still pinned by full
commit SHA, which satisfies the hash-pinning requirement this issue is actually
about. Both are the node20 builds rather than the node16 defaults, so nothing
depends on a node16 runtime:
- upload-artifact -> c6a3b2bd (v3.2.2-node20)
- download-artifact -> ad191675 (v3.1.0-node20)
Round 2 probes: the two fallback v3 builds in case the node20 ones do not
resolve, plus a producer/consumer pair that rehearses the deploy job -- same
pinned node image, same pinned download action, same pinned wrangler version,
stopping short of `wrangler pages deploy` so it touches nothing external.
Round 2 (602fd60) put the build job green:
check / check success 6s
Build and Deploy .../ build success 20s <- green
Build and Deploy .../ deploy skipped <- if: guard
probe / q1-upload-v3-node16 success 7s
probe / q2-upload-v3-node20 success 22s
probe / q3-build-for-roundtrip success 11s
probe / q4-deploy-dryrun failure 43s
Every v3 upload works and the build job is fixed. But q4 -- the deploy-side
rehearsal, which downloads the artifact in the pinned node container and
installs the pinned wrangler, stopping short of the publish call -- failed.
That is a break the deploy job would have hit on main, in a job nobody has
ever been able to run.
q4 bundled two things together, so round 3 splits them:
- r1 runs only the wrangler install and invocation. Worth measuring rather
than assuming: wrangler 4.120.0 declares engines.node >= 22 and the deploy
container is node 20, though the pre-issue deploy did run an unpinned
wrangler on node:20 successfully.
- r2a/r2b run the artifact round trip with no wrangler at all.
- r3a/r3b do the same for the newer node20 artifact builds, so the choice
between the two pairs is made on measurement.
deploy.yml meanwhile moves to the artifact commits that the mutable `@v3`
references were actually resolving to while this site was deploying, rather
than to the newest thing on the v3 line:
- upload-artifact -> ff15f030 (v3.2.1)
- download-artifact -> 9bc31d5c (v3.0.2)
That is the conservative reading of what this issue is for: pin what is known
to work, do not take a version bump for free on the way past.
Round 3 (07af755) cleared the artifact path and left one failure:
check / check success 8s
Build and Deploy .../ build success 8s <- green
Build and Deploy .../ deploy skipped <- if: guard
probe / r1-wrangler-only failure 7s
probe / r2a-upload-proven success 12s
probe / r2b-download-proven success 2s
probe / r3a-upload-node20 success 8s
probe / r3b-download-node20 success 2s
r2a/r2b and r3a/r3b upload and download the real site tarball across the two
job containers, so the artifact round trip is sound. r1 does nothing but
install wrangler and invoke it, and it fails.
Reproduced locally in the pinned node image, which is faster than another CI
round:
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler@4.120.0; wrangler --version'
install exit=0 (with EBADENGINE warnings)
Wrangler requires at least Node.js v22.0.0. You are using v20.20.2.
version exit=1
npm treats engines as a warning on an explicit version, so the install step
would have passed and the deploy step would have failed -- a second break,
independent of the artifact one, in the same job nobody could run.
The instructive part is what the unpinned command it replaced was doing:
$ docker run --rm node@sha256:8f693eaa... sh -c \
'npm install -g wrangler; wrangler --version'
`-- wrangler@4.86.0
4.86.0
npm resolves a bare name to the newest version whose engines the running node
satisfies, so `npm install -g wrangler` on node 20 has been installing 4.86.0,
not the 4.120.0 that `latest` points at. Pinning 4.120.0 was therefore not
"pin the version we are already getting", it was an unnoticed major-ish bump
onto a node the container does not have.
So this pins wrangler 4.86.0 (engines: node >= 20.3.0, published 2026-04-28),
which is exactly the version that has been deploying this site, verified to
install and run on the pinned node 20 digest. The node image digest is left
alone. Bumping the container to node 22 to keep 4.120.0 is the alternative,
but that changes the deploy runtime for no benefit this issue asks for.
Round 4 replaces the probe jobs with a single end-to-end rehearsal: the build
job as written, then the deploy job as written with `wrangler pages deploy
--help` in place of the publish call.
deploy.yml was the last file in the repo carrying mutable external references.
Both job container images are now pinned by digest, all three `uses:` by a full
40-hex commit SHA, and the wrangler install by exact version, each with a
version/date comment above the reference.
- build container: klakegg/hugo:ext-alpine (abandoned since 2021, mutable tag)
replaced by the exact alpine 3.21 digest the Dockerfile already pins, with a
pre-checkout `apk add --no-cache nodejs git tar` step, `shell: sh` as the job
default, then script/bootstrap and script/test. One pinned base and one
dependency list now serve both the check build and the deploy build.
- deploy container: node:20 -> node@sha256:8f693eaa... (node 20.20.2 bookworm).
- actions/checkout: v4 -> 11bd7190... (v4.2.2), the same SHA check.yml pins.
- actions/upload-artifact: -> ff15f030... (v3.2.1).
- actions/download-artifact: -> 9bc31d5c... (v3.0.2).
- wrangler: `npm install -g wrangler` -> `wrangler@4.86.0`.
Also drops the dead feat/initial-site push trigger, reindents to 4-space YAML
to match check.yml, and adds `if: github.ref_name == 'main'` to the deploy job
so it can never publish from a branch.
This is the second attempt. The first passed two adversarial reviews, merged,
and broke the deploy, because deploy.yml triggers only on push to main and so
nobody could execute what they were reviewing. This time the workflow was
temporarily triggered on the branch, with the deploy job guarded off, and
iterated against the commit-status API until the build job ran green for real.
Doing that found two independent breaks that review had not:
1. actions/upload-artifact v4 fails on this Gitea Actions instance -- artifacts
v4 is a different wire protocol and it is not served here. Two otherwise
identical branch jobs, one with the v4 upload step and one without, failed
and passed respectively. The issue asked for the v3 -> v4 bump; the
artifact actions instead stay on the v3 line, pinned by SHA, at the exact
commits the mutable @v3 references were already resolving to. Tracked
separately in issue 20.
2. wrangler 4.120.0 requires node >= 22 and refuses to start on the pinned
node 20 container. `npm install` only warns about engines, so the install
step would have passed and the deploy step would have failed. The unpinned
command this replaces was never installing `latest` either: npm resolves a
bare name to the newest version whose engines the running node satisfies,
which on node 20 is 4.86.0. So 4.86.0 is what has actually been deploying
this site, and that is what is pinned. Tracked separately in issue 21.
The temporary branch trigger and the temporary probe workflow used to bisect
this are removed in this commit; the deploy guard is deliberately kept.
Verified: make check and script/cibuild green; the build job observed green on
the branch under act_runner (commit 73f912c, "Successful in 7s"); a probe job
pair rehearsed the deploy job end to end -- same pinned node image, same pinned
download action, same pinned wrangler, real site tarball extracted -- stopping
at `wrangler pages deploy --help` instead of publishing. The real deploy job
remains unexercised: it needs CLOUDFLARE_API_TOKEN and would publish, so it can
only run on main. The main run must still be watched and the live site
confirmed.
clawbot
self-assigned this 2026-08-09 05:09:23 +02:00
Two files against main, .gitea/workflows/deploy.yml and TODO.md.
The pinned values, all with a version/date comment above the reference:
Reference
Pinned to
build container
alpine@sha256:c3f8e73f... (byte-identical to Dockerfile line 6)
deploy container
node@sha256:8f693eaa... (node 20.20.2 bookworm)
actions/checkout
11bd71901bbe5b1630ceea73d27597364c9af683 (v4.2.2, same SHA check.yml uses)
actions/upload-artifact
ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5 (v3.2.1)
actions/download-artifact
9bc31d5ccc31df68ecc42ccf4149144866c47d8a (v3.0.2)
wrangler
wrangler@4.86.0
Plus: klakegg/hugo:ext-alpine gone; a pre-checkout apk add --no-cache nodejs git tar step and shell: sh job default so the
runner can execute JS actions on a bare musl image; script/bootstrap then script/test; the dead feat/initial-site trigger dropped; 4-space YAML; and if: github.ref_name == 'main' on the deploy job, 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.yml of single-hypothesis jobs,
since each job is its own status context and the jobs/logs API is not
readable 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 / check is green on it.
Local, in the pinned images (diagnosis, not a build path):
docker run in node@sha256:8f693eaa...: wrangler@4.120.0 installs with
EBADENGINE warnings and then exits 1 with "Wrangler requires at least Node.js
v22.0.0"; unpinned npm install -g wrangler resolves to 4.86.0; explicit wrangler@4.86.0 installs and runs.
Repo gates: make check and script/cibuild both green on the final tree.
What a reviewer should push on
The v3 artifact actions. They are deprecated upstream and the issue asked for
v4. My case is that v4 measurably fails here (p2 vs p4) and that the SHAs
chosen are the exact commits @v3 was already resolving to while the site
deployed. If you think a deprecated-but-working pin is the wrong trade, that
is the argument to have.
The wrangler downgrade from latest. 4.86.0 is what npm was already
installing on node 20; the alternative is bumping the container to node 22
(#21), which I judged out of scope here.
Whether submodules: recursive and the git package earn their place --
there is no .gitmodules in the repo. I left both as-is because they came
from 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 deploy job has never executed. It needs CLOUDFLARE_API_TOKEN and it
publishes. Every step of it is rehearsed by s2-deploy-dryrun except the final wrangler pages deploy call and the Cloudflare API behind it. The main run
must be watched after merge and https://lora.vegas/ confirmed. I am not
claiming more than that -- overclaiming verification is what shipped the last
break.
Summary for review
==================
What changed
------------
Two files against `main`, `.gitea/workflows/deploy.yml` and `TODO.md`.
The pinned values, all with a version/date comment above the reference:
| Reference | Pinned to |
| --- | --- |
| `build` container | `alpine@sha256:c3f8e73f...` (byte-identical to `Dockerfile` line 6) |
| `deploy` container | `node@sha256:8f693eaa...` (node 20.20.2 bookworm) |
| `actions/checkout` | `11bd71901bbe5b1630ceea73d27597364c9af683` (v4.2.2, same SHA `check.yml` uses) |
| `actions/upload-artifact` | `ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5` (v3.2.1) |
| `actions/download-artifact` | `9bc31d5ccc31df68ecc42ccf4149144866c47d8a` (v3.0.2) |
| wrangler | `wrangler@4.86.0` |
Plus: `klakegg/hugo:ext-alpine` gone; a pre-checkout
`apk add --no-cache nodejs git tar` step and `shell: sh` job default so the
runner can execute JS actions on a bare musl image; `script/bootstrap` then
`script/test`; the dead `feat/initial-site` trigger dropped; 4-space YAML; and
`if: github.ref_name == 'main'` on the `deploy` job, 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:
2d328e7 build failure 15s reproduced the main failure
602fd60 build success 20s
07af755 build success 8s
73f912c build success 7s overall: success, incl. deploy rehearsal
The bisect was done with a temporary `probe.yml` of single-hypothesis jobs,
since each job is its own status `context` and the jobs/logs API is not
readable 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 / check` is green on it.
Local, in the pinned images (diagnosis, not a build path):
- `docker run` in `node@sha256:8f693eaa...`: `wrangler@4.120.0` installs with
EBADENGINE warnings and then exits 1 with "Wrangler requires at least Node.js
v22.0.0"; unpinned `npm install -g wrangler` resolves to 4.86.0; explicit
`wrangler@4.86.0` installs and runs.
Repo gates: `make check` and `script/cibuild` both green on the final tree.
What a reviewer should push on
------------------------------
- The v3 artifact actions. They are deprecated upstream and the issue asked for
v4. My case is that v4 measurably fails here (p2 vs p4) and that the SHAs
chosen are the exact commits `@v3` was already resolving to while the site
deployed. If you think a deprecated-but-working pin is the wrong trade, that
is the argument to have.
- The wrangler downgrade from `latest`. 4.86.0 is what npm was already
installing on node 20; the alternative is bumping the container to node 22
(#21), which I judged out of scope here.
- Whether `submodules: recursive` and the `git` package earn their place --
there is no `.gitmodules` in the repo. I left both as-is because they came
from 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 `deploy` job has never executed. It needs `CLOUDFLARE_API_TOKEN` and it
publishes. Every step of it is rehearsed by `s2-deploy-dryrun` except the final
`wrangler pages deploy` call and the Cloudflare API behind it. The `main` run
must be watched after merge and https://lora.vegas/ confirmed. I am not
claiming more than that -- overclaiming verification is what shipped the last
break.
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 @v3 refs currently
resolve 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.
Swept all 498 stable versions in the registry: 4.86.0 is the highest stable
version whose engines.node admits node 20. Every release from 4.113.0
through 4.120.0 declares >=22.0.0. So the claim that a bare npm install -g wrangler on 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.
Both match the PR body exactly, including the absence of a build status on
the 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 54ed637 touches 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 change
to deploy.yml between the commit that ran green on a real act_runner and
the commit proposed for merge. The green build result therefore transfers to
the final tree. Had a single uses:, image:, or run: line moved after 73f912c, 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 contains check.yml and deploy.yml only. probe.yml is present at 73f912c and absent at head. Confirmed by git ls-tree at both commits.
YAML parses. on is exactly {'push': {'branches': ['main']}} — the
temporary branch trigger is gone from the parsed structure, not merely
commented out.
deploy.if parses as github.ref_name == 'main'. Correct for a push to main, and it is defence in depth given the trigger is already [main]
only. Kept permanently, as requested in issue #7 comment 46551.
Alpine digest is byte-identical to Dockerfile line 6: c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709. Compared
character by character, not by prefix.
Node digest resolves to a running node 20.20.2, as the comment claims
(verified by executing the image).
actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 is identical to
the SHA .gitea/workflows/check.yml already pins; the two workflows agree.
Indentation: no line in deploy.yml begins at a 2/6/10-space level. 4-space
throughout, matching check.yml and .editorconfig. DoD items 5 and 6 met.
TODO.md is 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.
Landing commit title is Hash-pin every external reference in deploy.yml (closes #7). Exactly one commit in 3d17e22..HEAD carries a closes #
token, and it is the head commit. No intermediate commit closes the issue.
make check green (prettier clean: "All matched files use Prettier code
style!"; Hugo build succeeds). The two warnings emitted are pre-existing —
the languageCode deprecation and the taxonomy layout warning tracked in #13
— and are not introduced by this change.
script/cibuild green.
No Claude/Anthropic/attribution references anywhere: working tree (excluding .git and themes), the diff, all five commit messages and trailers, the PR
body, and the PR comment. Grep returned nothing.
Inclusive terminology: clean.
Scope: two files. Jobs stay separate, no renovate/dependabot, no package.json. No scope creep found.
Mergeable: head is a direct descendant of origin/main (3d17e22).
Fast-forward, no conflicts.
Follow-up issues #20 and #21 both exist, are open, and are correctly scoped
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 deploy job has never run, that it needs CLOUDFLARE_API_TOKEN and
publishes, that every step is individually rehearsed except the final wrangler pages deploy call and the Cloudflare API behind it, and that the main run must still be watched after merge with the live site confirmed. The
review comment repeats the same limitation. The unpinned apk packages are also
disclosed as an unclaimed gap with a tracking issue.
The distinction between "the deploy job ran" and "a probe job with the same
image, same pinned download action, same pinned wrangler and the real tarball
ran, stopping at --help" is drawn correctly and consistently everywhere it
appears. 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: recursive on the checkout step is a no-op: there is no .gitmodules in the repo and themes is an ordinary tree, not a gitlink.
The inline comment justifies the git apk package by that option
("git is needed for checkout's submodules: recursive"), so it justifies a
dependency by a feature the repo does not use. The git install is still
independently 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 their action.yml, while actions/checkout v4.2.2 declares node20. The current act_runner evidently executes them anyway (proven by build and s2-deploy-dryrun going green), and this is the status quo rather than a
regression, 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.
tar in the apk prerequisite step is arguably redundant — busybox provides
a tar sufficient for the tar -czf site.tar.gz public step. Harmless, and
part 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, 73f912c and 54ed637; the intermediate probe rounds
(2d328e7, 602fd60, 07af755) were not re-read from the API, since their
conclusions are all superseded by the two facts that were verified directly —
the final build result and the empty functional diff between the tested and
final commits. The deploy job itself cannot be exercised pre-merge and was
not; 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 main
run and confirm https://lora.vegas/ serves a fresh deploy, with an immediate
revert if the deploy job fails.
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.
actions/upload-artifact refs/tags/v3.2.1 -> ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5 (commit)
actions/upload-artifact refs/tags/v3 -> ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5 (commit)
actions/download-artifact refs/tags/v3.0.2 -> 9bc31d5ccc31df68ecc42ccf4149144866c47d8a (commit)
actions/download-artifact refs/tags/v3 -> 9bc31d5ccc31df68ecc42ccf4149144866c47d8a (commit)
Both SHAs are real commit objects, both genuinely correspond to the claimed v3
point releases, and — the load-bearing part — the mutable `@v3` refs currently
resolve 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:
wrangler 4.120.0 engines.node >=22.0.0 (also dist-tags.latest)
wrangler 4.86.0 engines.node >=20.3.0
Swept all 498 stable versions in the registry: **4.86.0 is the highest stable
version whose `engines.node` admits node 20.** Every release from 4.113.0
through 4.120.0 declares `>=22.0.0`. So the claim that a bare
`npm install -g wrangler` on 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`:
node --version -> v20.20.2
npm --version -> 10.8.2
npm install -g wrangler -> exit 0, `-- wrangler@4.86.0
wrangler --version -> 4.86.0, exit 0
npm install -g wrangler@4.120.0 -> exit 0 (install SUCCEEDS)
wrangler --version -> exit 1
"Wrangler requires at least Node.js v22.0.0. You are using v20.20.2."
npm install -g wrangler@4.86.0 -> exit 0
wrangler --version -> 4.86.0, exit 0
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:
check / check (push) success 6s
Build and Deploy to Cloudflare Pages / build (push) success 7s
Build and Deploy to Cloudflare Pages / deploy (push) skipped
probe / s1-build (push) success 13s
probe / s2-deploy-dryrun (push) success 10s
`54ed6376af8c5f27508f434ebd7aa7f053b7b2c6` — `state: success`, 1 status:
check / check (push) success 6s
Both match the PR body exactly, including the absence of a `build` status on
the 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 54ed637` touches 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 change
to `deploy.yml` between the commit that ran green on a real `act_runner` and
the commit proposed for merge.** The green `build` result therefore transfers to
the final tree. Had a single `uses:`, `image:`, or `run:` line moved after
`73f912c`, 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 contains `check.yml` and `deploy.yml` only.
`probe.yml` is present at `73f912c` and absent at head. Confirmed by
`git ls-tree` at both commits.
- YAML parses. `on` is exactly `{'push': {'branches': ['main']}}` — the
temporary branch trigger is gone from the parsed structure, not merely
commented out.
- `deploy.if` parses as `github.ref_name == 'main'`. Correct for a push to
`main`, and it is defence in depth given the trigger is already `[main]`
only. Kept permanently, as requested in issue #7 comment 46551.
- Alpine digest is byte-identical to `Dockerfile` line 6:
`c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709`. Compared
character by character, not by prefix.
- Node digest resolves to a running node 20.20.2, as the comment claims
(verified by executing the image).
- `actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683` is identical to
the SHA `.gitea/workflows/check.yml` already pins; the two workflows agree.
- Indentation: no line in `deploy.yml` begins at a 2/6/10-space level. 4-space
throughout, matching `check.yml` and `.editorconfig`. DoD items 5 and 6 met.
- `TODO.md` is 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.
- Landing commit title is `Hash-pin every external reference in deploy.yml
(closes #7)`. Exactly one commit in `3d17e22..HEAD` carries a `closes #`
token, and it is the head commit. No intermediate commit closes the issue.
- `make check` green (prettier clean: "All matched files use Prettier code
style!"; Hugo build succeeds). The two warnings emitted are pre-existing —
the `languageCode` deprecation and the taxonomy layout warning tracked in #13
— and are not introduced by this change.
- `script/cibuild` green.
- No Claude/Anthropic/attribution references anywhere: working tree (excluding
`.git` and `themes`), the diff, all five commit messages and trailers, the PR
body, and the PR comment. Grep returned nothing.
- Inclusive terminology: clean.
- Scope: two files. Jobs stay separate, no renovate/dependabot, no
`package.json`. No scope creep found.
- Mergeable: head is a direct descendant of `origin/main` (`3d17e22`).
Fast-forward, no conflicts.
- Follow-up issues #20 and #21 both exist, are open, and are correctly scoped
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 `deploy` job has never run, that it needs `CLOUDFLARE_API_TOKEN` and
publishes, that every step is individually rehearsed **except** the final
`wrangler pages deploy` call and the Cloudflare API behind it, and that the
`main` run must still be watched after merge with the live site confirmed. The
review comment repeats the same limitation. The unpinned apk packages are also
disclosed as an unclaimed gap with a tracking issue.
The distinction between "the `deploy` job ran" and "a probe job with the same
image, same pinned download action, same pinned wrangler and the real tarball
ran, stopping at `--help`" is drawn correctly and consistently everywhere it
appears. 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.
1. `submodules: recursive` on the checkout step is a no-op: there is no
`.gitmodules` in the repo and `themes` is an ordinary tree, not a gitlink.
The inline comment justifies the `git` apk package by that option
("git is needed for checkout's `submodules: recursive`"), so it justifies a
dependency by a feature the repo does not use. The `git` install is still
independently 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.
2. Both pinned artifact actions declare `runs.using: 'node16'` in their
`action.yml`, while `actions/checkout` v4.2.2 declares `node20`. The current
`act_runner` evidently executes them anyway (proven by `build` and
`s2-deploy-dryrun` going green), and this is the status quo rather than a
regression, 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.
3. `tar` in the apk prerequisite step is arguably redundant — busybox provides
a `tar` sufficient for the `tar -czf site.tar.gz public` step. Harmless, and
part 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, `73f912c` and `54ed637`; the intermediate probe rounds
(`2d328e7`, `602fd60`, `07af755`) were not re-read from the API, since their
conclusions are all superseded by the two facts that were verified directly —
the final `build` result and the empty functional diff between the tested and
final commits. The `deploy` job itself cannot be exercised pre-merge and was
not; 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 `main`
run and confirm https://lora.vegas/ serves a fresh deploy, with an immediate
revert if the `deploy` job fails.
Manager note — PASS accepted. Relabelled merge-ready and merging now.
main is unprotected in this repo, so this merges directly rather than being
assigned 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 54ed637 touches
only the temporary-trigger removal, the probe.yml deletion, three comment
rewordings, and TODO.md — zero functional change to deploy.yml between
the 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 pass
on main" are different claims, and the gap between them is exactly where this
issue 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 @v3 tags point at today — so the pins are
byte-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: recursive is a no-op (no .gitmodules; themes is a plain
tree, not a gitlink), which also undercuts the stated justification for the git package.
Both artifact v3 actions declare runs.using: 'node16', which works on the
current runner but is a durability risk.
apk add tar is 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 deploy job has still never executed. It needs CLOUDFLARE_API_TOKEN and publishes, so it cannot be exercised from a branch
under any arrangement. I am merging and watching the main run immediately,
with an immediate revert if deploy fails — same as last time, except that
this time the build job it depends on has been proven on a real runner.
Manager note — PASS accepted. Relabelled `merge-ready` and merging now.
`main` is unprotected in this repo, so this merges directly rather than being
assigned 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 54ed637` touches
only the temporary-trigger removal, the `probe.yml` deletion, three comment
rewordings, and `TODO.md` — **zero functional change to `deploy.yml` between
the 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 pass
on `main`" are different claims, and the gap between them is exactly where this
issue 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 `@v3` tags point at today — so the pins are
byte-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:
1. `submodules: recursive` is a no-op (no `.gitmodules`; `themes` is a plain
tree, not a gitlink), which also undercuts the stated justification for the
`git` package.
2. Both artifact v3 actions declare `runs.using: 'node16'`, which works on the
current runner but is a durability risk.
3. `apk add tar` is 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 `deploy` job has still never executed. It needs
`CLOUDFLARE_API_TOKEN` and publishes, so it cannot be exercised from a branch
under any arrangement. I am merging and watching the `main` run immediately,
with an immediate revert if `deploy` fails — same as last time, except that
this time the `build` job it depends on has been proven on a real runner.
clawbot
merged commit 9959cb5794 into main2026-08-09 07:03:41 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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.