Hash-pin every external reference in deploy.yml, verified on a real runner (closes #7) #22

Merged
clawbot merged 5 commits from pin-deploy-refs-observable into main 2026-08-09 07:03:41 +02:00
Collaborator

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: 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, 73f912ceverything 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.
clawbot added the needs-review label 2026-08-09 05:09:18 +02:00
clawbot added 5 commits 2026-08-09 05:09:18 +02:00
Re-apply deploy.yml pinning behind a deploy guard, and probe the failure
Some checks failed
check / check (push) Successful in 10s
Build and Deploy to Cloudflare Pages / build (push) Failing after 15s
probe / p1-bare-alpine-checkout (push) Failing after 3s
probe / p2-alpine-apk-checkout (push) Successful in 5s
probe / p3-alpine-apk-build (push) Successful in 15s
probe / p4-alpine-apk-upload (push) Failing after 11s
probe / p5-node20alpine-checkout (push) Successful in 8s
probe / p6-node20slim-checkout (push) Successful in 11s
Build and Deploy to Cloudflare Pages / deploy (push) Has been skipped
2d328e759b
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.
Pin the artifact actions on v3: v4 does not work on this instance
Some checks failed
check / check (push) Successful in 6s
Build and Deploy to Cloudflare Pages / build (push) Successful in 20s
probe / q1-upload-v3-node16 (push) Successful in 7s
probe / q2-upload-v3-node20 (push) Successful in 22s
probe / q3-build-for-roundtrip (push) Successful in 11s
Build and Deploy to Cloudflare Pages / deploy (push) Has been skipped
probe / q4-deploy-dryrun (push) Failing after 43s
602fd609e7
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.
Move the artifact pair to the exact commits @v3 was resolving to
Some checks failed
check / check (push) Successful in 8s
Build and Deploy to Cloudflare Pages / build (push) Successful in 8s
probe / r1-wrangler-only (push) Failing after 7s
probe / r2a-upload-proven (push) Successful in 12s
probe / r3a-upload-node20 (push) Successful in 8s
Build and Deploy to Cloudflare Pages / deploy (push) Has been skipped
probe / r2b-download-proven (push) Successful in 2s
probe / r3b-download-node20 (push) Successful in 2s
07af755d1e
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.
Pin wrangler to the version that actually runs on the pinned node image
All checks were successful
check / check (push) Successful in 6s
Build and Deploy to Cloudflare Pages / build (push) Successful in 7s
probe / s1-build (push) Successful in 13s
Build and Deploy to Cloudflare Pages / deploy (push) Has been skipped
probe / s2-deploy-dryrun (push) Successful in 10s
73f912c7ed
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.
Hash-pin every external reference in deploy.yml (closes #7)
All checks were successful
check / check (push) Successful in 6s
54ed6376af
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
Author
Collaborator

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.

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.
Author
Collaborator

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 -&gt; ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5  (commit)
actions/upload-artifact   refs/tags/v3     -&gt; ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5  (commit)
actions/download-artifact refs/tags/v3.0.2 -&gt; 9bc31d5ccc31df68ecc42ccf4149144866c47d8a  (commit)
actions/download-artifact refs/tags/v3     -&gt; 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 &gt;=22.0.0    (also dist-tags.latest)
wrangler  4.86.0  engines.node &gt;=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 &gt;=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                       -&gt; v20.20.2
npm --version                        -&gt; 10.8.2
npm install -g wrangler              -&gt; exit 0, `-- wrangler@4.86.0
wrangler --version                   -&gt; 4.86.0, exit 0
npm install -g wrangler@4.120.0      -&gt; exit 0     (install SUCCEEDS)
wrangler --version                   -&gt; exit 1
   "Wrangler requires at least Node.js v22.0.0. You are using v20.20.2."
npm install -g wrangler@4.86.0       -&gt; exit 0
wrangler --version                   -&gt; 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.

73f912c7edec21a92ba298d491b872af7e48f75bstate: 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

54ed6376af8c5f27508f434ebd7aa7f053b7b2c6state: 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.

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 -&gt; ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5 (commit) actions/upload-artifact refs/tags/v3 -&gt; ff15f0306b3f739f7b6fd43fb5d26cd321bd4de5 (commit) actions/download-artifact refs/tags/v3.0.2 -&gt; 9bc31d5ccc31df68ecc42ccf4149144866c47d8a (commit) actions/download-artifact refs/tags/v3 -&gt; 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 -&gt; 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 &gt;=22.0.0 (also dist-tags.latest) wrangler 4.86.0 engines.node &gt;=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 `&gt;=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 -&gt; v20.20.2 npm --version -&gt; 10.8.2 npm install -g wrangler -&gt; exit 0, `-- wrangler@4.86.0 wrangler --version -&gt; 4.86.0, exit 0 npm install -g wrangler@4.120.0 -&gt; exit 0 (install SUCCEEDS) wrangler --version -&gt; exit 1 "Wrangler requires at least Node.js v22.0.0. You are using v20.20.2." npm install -g wrangler@4.86.0 -&gt; exit 0 wrangler --version -&gt; 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.
clawbot added merge-ready and removed needs-review labels 2026-08-09 07:03:23 +02:00
Author
Collaborator

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.mdzero 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.

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 main 2026-08-09 07:03:41 +02:00
Sign in to join this conversation.