ci(deno.sh): losing git is an error, one digest, and no image left behind #42

julian · 2026-08-16T13:54:53+02:00

julian commented

2026-08-16 13:54:53 +02:00

Closes #29 — all three follow-ups, one pass over .forgejo/deno.sh as the issue framed it.

1. The soft fallback re-opened the silent-skip hole

deno.sh documents that without git, docker/backend/commit_test.ts "silently never ran at all", and that test still guards itself with ignore: !hasGit. The fallback added in #27 warned on stderr and set CI_IMAGE=$IMAGE, so an unreachable Debian mirror put that suite straight back to skipping with CI green. The external-module suites do fail loudly — but only because they shell out to git unguarded, which is the accident of which suite happens to exist, not a property of the arrangement.

Of the issue's two options, this is the second: the fallback is now opt-in.

env:
  DENO_GIT: optional

Everything else fails, with the reason and the escape hatch both named:

could not build opsdeck-deno-ci:t…, and git is not optional for this call.
Set DENO_GIT=optional if this command does not need git — a run
without it turns the git-guarded suites into silent skips.

deps.yml sets it on all three steps — none of those commands pass --allow-run, so they could not spawn git if they tried. build.yml deliberately does not: deno task ci is what runs the suites that need the real thing.

tools/ci_pins_test.ts pins that wiring: every deno.sh step in deps.yml carries the opt-out — comments stripped first, so it has to be set, not mentioned, and steps split on ANY key that can open a list item (- if:, - env:, - id:, a bare - run:), so a step cannot slip past the check by merging into its predecessor's chunk, wherever it sits in the file. The opt-out's value must also end at optional: DENO_GIT: optionally is not optional to the wrapper, so it reads as missing rather than set. No other workflow names DENO_GIT anywhere in its text, in any spelling. The workflow list is discovered from .forgejo/workflows/ rather than hardcoded, so a new workflow cannot opt out silently (or ship an unpinned action or its own Deno image); it earns the opt-out by joining the per-step check deps.yml is under.

The first option (a "git was expected" variable that turns hasGit skips into failures) would have meant editing the test files, against the issue's own framing that one pass over deno.sh closes all three.

2. Nothing removed superseded derived images

Prune the other opsdeck-deno-ci:* tags on every run that holds a derived image — cache hit and cold build alike, via one prune_stale() both paths call. A sweep only after a build always arrived one generation early: the tag a build supersedes was needed until that day, so it is young by the gate below, and no pass came past again until the next recipe change — the runner steady-stated at current + previous. Swept on the cached path too, a tag stops being refreshed two days after its last use and the next run of anything collects it. Best effort — another job may be running one, and losing that race is not this run's problem; running from the cached path also narrows the reverse race, since a candidate is by definition something nothing has self-tagged for two days. The tag just built and any <none> are skipped, and the query is scoped to the opsdeck-deno-ci repository so the base denoland/deno image is never a candidate.

Only tags this runner has neither built nor tagged for two days are candidates. The gate is LastTagTime: the build moves it, the post-eviction rebuild moves it, and a cache hit moves it too — the wrapper self-tags (docker tag X X) on the cached path, since nothing else there touches a timestamp — so a recipe in use is young by definition and never evicted. Created cannot express that: a cache-hit rebuild hands back the cached image with it untouched, so an in-use recipe would read as old as its first build, be evicted anyway, and come back still "old". LastTagTime is per image, not per tag — two tags on one ID share the timestamp — which over-keeps and never over-prunes. A daemon that cannot answer it (the containerd image store can hand back the zero time) keeps everything and warns once, rather than being a prune that silently never prunes — green with the work quietly not done is the exact shape item 1 exists against.

3. `RECIPE_KEY` was not stable across runners

RECIPE_KEY=$(printf '%s' "$RECIPE" | { sha256sum 2>/dev/null || cksum; } | tr -cd '[:alnum:]' | cut -c1-16)

A runner without sha256sum produced a different tag for the same recipe, so a mixed fleet built the same image twice and cached neither for the other — and cksum is a CRC32 where everything around it reasons about a hash. One digest now, and a runner without it says so rather than substituting another — unless the caller set DENO_GIT=optional, in which case it needs no derived image at all and degrades the same way a failed build does: warn, run the base image. The hatches compose.

no sha256sum on this runner: the derived-image tag has to be the
same value on every runner, so there is no second digest to use.

Verification

No docker daemon on the machine this was written on, so the script was exercised against a stubbed docker CLI that logs every call and fails on demand; the daemon semantics this arrangement leans on (Created frozen across cache-hit rebuilds, self-tag and cache-hit rebuild moving LastTagTime, {{.Metadata.LastTagTime.Unix}} rendering, the cold path end-to-end) are the reviewer's measurements against real daemons in rounds 2–4.

Stub matrix this round, 28 checks: a cache hit self-tags exactly once, prunes a stale sibling, no build; a young sibling is kept; a cold build prunes only the tag older than PRUNE_AFTER (48 h) and skips <none> and the tag just built; two candidates the daemon cannot age warn exactly once, keep everything, rc=0 — the warned flag lives in the prune pipeline's subshell, which is one subshell for the whole loop, so it persists across iterations (measured: two candidates, one warning line); a negative zero-time answer is kept and warned; docker images, rmi and tag failures never red a run; a failing build without the opt-out is the only rc=1 in the matrix; both hatches unchanged in both directions (build-fail × DENO_GIT, missing sha256sum × DENO_GIT). The recipe text is untouched by this round, so the tag stays tf4224312806e885c by construction and no fleet rebuild rides along.

Sixteen mutations against tools/ci_pins_test.ts, all landing in the demanded direction: steps opening with if:, env: (holding another variable) and id:, an unnamed - run:, a comment-only opt-out, the if:-first step placed directly after Checkout (position no longer matters), DENO_GIT: optionally and inline DENO_GIT=optionally all go red; the quoted DENO_GIT: "optional", the inline DENO_GIT=optional run line, and a new nightly.yml calling deno.sh without the opt-out stay green; consolidating deps.yml's envs to job level stays the documented false red; build.yml step env, a build.yml comment naming DENO_GIT, and a nightly.yml opting out all red.

Repository gates: deno task check, deno lint (196 files), deno fmt --check (314 files), tools/ (58 passed: ci_pins_test.ts + dep-check), sh -n .forgejo/deno.sh. packages/server/tests/ was not re-run this round — the diff (CI script, test over YAML, docs) touches no server code; round 1's run stands (109 passed, 1 pre-existing interval leak in external_modules_test.ts).

docs/dependencies.md carries the git policy, the tagging, the pruning — now "every run that holds a derived image", with the one-generation-late reasoning and the warn-once degradation — and what the test asserts under the deno.sh section.

Closes #29 — all three follow-ups, one pass over `.forgejo/deno.sh` as the issue framed it. ## 1. The soft fallback re-opened the silent-skip hole `deno.sh` documents that without git, `docker/backend/commit_test.ts` "silently never ran at all", and that test still guards itself with `ignore: !hasGit`. The fallback added in #27 warned on stderr and set `CI_IMAGE=$IMAGE`, so an unreachable Debian mirror put that suite straight back to skipping with CI green. The external-module suites do fail loudly — but only because they shell out to git unguarded, which is the accident of which suite happens to exist, not a property of the arrangement. Of the issue's two options, this is the second: the fallback is now opt-in. ```yaml env: DENO_GIT: optional ``` Everything else fails, with the reason and the escape hatch both named: ``` could not build opsdeck-deno-ci:t…, and git is not optional for this call. Set DENO_GIT=optional if this command does not need git — a run without it turns the git-guarded suites into silent skips. ``` `deps.yml` sets it on all three steps — none of those commands pass `--allow-run`, so they could not spawn git if they tried. `build.yml` deliberately does not: `deno task ci` is what runs the suites that need the real thing. `tools/ci_pins_test.ts` pins that wiring: every `deno.sh` step in `deps.yml` carries the opt-out — comments stripped first, so it has to be **set**, not mentioned, and steps split on ANY key that can open a list item (`- if:`, `- env:`, `- id:`, a bare `- run:`), so a step cannot slip past the check by merging into its predecessor's chunk, wherever it sits in the file. The opt-out's value must also **end** at `optional`: `DENO_GIT: optionally` is not `optional` to the wrapper, so it reads as missing rather than set. No other workflow names `DENO_GIT` anywhere in its text, in any spelling. The workflow list is discovered from `.forgejo/workflows/` rather than hardcoded, so a new workflow cannot opt out silently (or ship an unpinned action or its own Deno image); it earns the opt-out by joining the per-step check `deps.yml` is under. The first option (a "git was expected" variable that turns `hasGit` skips into failures) would have meant editing the test files, against the issue's own framing that one pass over `deno.sh` closes all three. ## 2. Nothing removed superseded derived images Prune the other `opsdeck-deno-ci:*` tags on **every run that holds a derived image** — cache hit and cold build alike, via one `prune_stale()` both paths call. A sweep only after a build always arrived one generation early: the tag a build supersedes was needed until that day, so it is young by the gate below, and no pass came past again until the next recipe change — the runner steady-stated at current + previous. Swept on the cached path too, a tag stops being refreshed two days after its last use and the next run of anything collects it. Best effort — another job may be running one, and losing that race is not this run's problem; running from the cached path also narrows the reverse race, since a candidate is by definition something nothing has self-tagged for two days. The tag just built and any `<none>` are skipped, and the query is scoped to the `opsdeck-deno-ci` repository so the base `denoland/deno` image is never a candidate. Only tags this runner has neither built nor tagged for two days are candidates. The gate is `LastTagTime`: the build moves it, the post-eviction rebuild moves it, and a cache hit moves it too — the wrapper self-tags (`docker tag X X`) on the cached path, since nothing else there touches a timestamp — so a recipe in use is young by definition and never evicted. `Created` cannot express that: a cache-hit rebuild hands back the cached image with it untouched, so an in-use recipe would read as old as its first build, be evicted anyway, and come back still "old". `LastTagTime` is per image, not per tag — two tags on one ID share the timestamp — which over-keeps and never over-prunes. A daemon that cannot answer it (the containerd image store can hand back the zero time) keeps everything and warns once, rather than being a prune that silently never prunes — green with the work quietly not done is the exact shape item 1 exists against. ## 3. `RECIPE_KEY` was not stable across runners ```sh RECIPE_KEY=$(printf '%s' "$RECIPE" | { sha256sum 2>/dev/null || cksum; } | tr -cd '[:alnum:]' | cut -c1-16) ``` A runner without `sha256sum` produced a different tag for the same recipe, so a mixed fleet built the same image twice and cached neither for the other — and `cksum` is a CRC32 where everything around it reasons about a hash. One digest now, and a runner without it says so rather than substituting another — unless the caller set `DENO_GIT=optional`, in which case it needs no derived image at all and degrades the same way a failed build does: warn, run the base image. The hatches compose. ``` no sha256sum on this runner: the derived-image tag has to be the same value on every runner, so there is no second digest to use. ``` ## Verification No docker daemon on the machine this was written on, so the script was exercised against a **stubbed docker CLI** that logs every call and fails on demand; the daemon semantics this arrangement leans on (`Created` frozen across cache-hit rebuilds, self-tag and cache-hit rebuild moving `LastTagTime`, `{{.Metadata.LastTagTime.Unix}}` rendering, the cold path end-to-end) are the reviewer's measurements against real daemons in rounds 2–4. Stub matrix this round, 28 checks: a cache hit self-tags exactly once, prunes a stale sibling, no build; a young sibling is kept; a cold build prunes only the tag older than `PRUNE_AFTER` (48 h) and skips `<none>` and the tag just built; two candidates the daemon cannot age warn **exactly once**, keep everything, rc=0 — the `warned` flag lives in the prune pipeline's subshell, which is one subshell for the whole loop, so it persists across iterations (measured: two candidates, one warning line); a negative zero-time answer is kept and warned; `docker images`, `rmi` and `tag` failures never red a run; a failing build without the opt-out is the only rc=1 in the matrix; both hatches unchanged in both directions (build-fail × `DENO_GIT`, missing `sha256sum` × `DENO_GIT`). The recipe text is untouched by this round, so the tag stays `tf4224312806e885c` by construction and no fleet rebuild rides along. Sixteen mutations against `tools/ci_pins_test.ts`, all landing in the demanded direction: steps opening with `if:`, `env:` (holding another variable) and `id:`, an unnamed `- run:`, a comment-only opt-out, the `if:`-first step placed directly after Checkout (position no longer matters), `DENO_GIT: optionally` and inline `DENO_GIT=optionally` all go red; the quoted `DENO_GIT: "optional"`, the inline `DENO_GIT=optional` run line, and a new `nightly.yml` calling `deno.sh` **without** the opt-out stay green; consolidating `deps.yml`'s envs to job level stays the documented false red; `build.yml` step env, a `build.yml` comment naming `DENO_GIT`, and a `nightly.yml` opting out all red. Repository gates: `deno task check`, `deno lint` (196 files), `deno fmt --check` (314 files), `tools/` (58 passed: `ci_pins_test.ts` + dep-check), `sh -n .forgejo/deno.sh`. `packages/server/tests/` was not re-run this round — the diff (CI script, test over YAML, docs) touches no server code; round 1's run stands (109 passed, 1 pre-existing interval leak in `external_modules_test.ts`). `docs/dependencies.md` carries the git policy, the tagging, the pruning — now "every run that holds a derived image", with the one-generation-late reasoning and the warn-once degradation — and what the test asserts under the `deno.sh` section.

julian added 1 commit

2026-08-16 13:54:53 +02:00

ci(deno.sh): losing git is an error, one digest, and no image left behind

Build and Deploy / verify (pull_request) Successful in 1m29s

Details

Build and Deploy / build (pull_request) Has been skipped

Details

Dependency Check / dependencies (pull_request) Successful in 1m16s

Details

72641eb21d

Three follow-ups from the review of #27, all in the one file.

The soft fallback re-opened the hole this file complains about. `deno.sh`
documents that without git `docker/backend/commit_test.ts` "silently never ran
at all", and that test still guards itself with `ignore: !hasGit` — so a
failed derived-image build that warned and carried on with the base image put
that suite straight back to skipping, with CI green. The external-module
suites do fail loudly, but only because they shell out to git unguarded: an
accident of which suite exists, not a property of the design.

So the fallback is opt-in. `DENO_GIT=optional` is for the caller that knows it
needs no git, and everything else fails with the reason and the escape hatch
named. `deps.yml` sets it on all three steps — dep-check reads files and talks
to registries. `build.yml` does not: `deno task ci` is what runs the suites
that need the real thing.

Nothing removed the superseded derived images, so a long-lived runner kept a
full Deno image per recipe or toolchain change. Prune the other
`opsdeck-deno-ci:*` tags after a successful build, tolerating a failure —
another job may be running one, and losing that race is not this run's
problem. `<none>` and the tag just built are skipped.

`RECIPE_KEY` fell back from `sha256sum` to `cksum`, so the same recipe keyed
to a different tag depending on which runner computed it: a mixed fleet built
the same image twice and cached neither for the other, and `cksum` is a CRC32
where the surrounding comment reasons about a hash. One digest now, and a
runner without it says so instead of substituting another.

Verified against a stubbed docker CLI: cached image, build-then-prune, prune
skipping both the current tag and `<none>`, a tolerated `rmi` failure, the
hard failure and the opt-in fallback, a stable key across runs, and a bumped
toolchain pin producing a different key. The missing-sha256sum path was run
with a PATH that lacks it.

Closes #29

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

julian requested review from thisilike

2026-08-16 13:54:56 +02:00

thisilike requested changes

2026-08-16 21:10:16 +02:00

Dismissed

thisilike left a comment

I re-ran the verification independently — my own stub docker on PATH, a throwaway docker/Dockerfile carrying the current pin, and the script exactly as it stands at 72641eb — rather than taking the table in the description on trust. Every one of its eight rows reproduces, so the mechanics are right. What I want changed is where two of the three items interact, and one place where the two escape hatches don't compose.

What holds up

The tag does not move. sha256sum | cut -c1-16 and main's sha256sum 2>/dev/null || cksum → tr -cd '[:alnum:]' → cut -c1-16 both produce f4224312806e885c for the current recipe. Dropping the tr is safe — cut -c1-16 never reaches the - suffix — and it matters that it is safe: every runner that already holds the derived image keeps its cache across this merge. Worth having checked, because a silent tag change here would have been a fleet-wide rebuild disguised as a cleanup.
The prune is correctly scoped and correctly cowardly. Cold build with two stale tags: both rmi'd, the tag just built skipped, denoland/deno never a candidate. rmi returning non-zero does not stop the run. Cached path does no build and no prune.
set -e does not fire on the prune pipeline. The while exits 0 whether the last body execution was continue or docker rmi … || true, so a run whose newest listed tag happens to be $CI_IMAGE does not die on the loop's status. That is load-bearing and easy to get wrong; it is right here.
case $stale in "$CI_IMAGE" | … — $CI_IMAGE is quoted so the tag is a literal, and the case word needs no quoting. Fine.
build.yml deliberately left at the default. deno task ci is the caller that runs the git-guarded suites, and it is the one that now fails hard. That is the whole point of the change and it is wired correctly.

1. The prune and the hard failure compound into cache thrash

Both workflows are runs-on: cth-ubuntu-latest, and the recipe is a function of the Dockerfile's FROM. So the moment two branches disagree on the Deno pin — which is precisely what a dep-check bump PR is, and filing those is that tool's entire job — the shared runner alternates:

PR push → verify builds recipe B, prunes A
main push → verify misses A (just pruned), rebuilds A, prunes B
next PR push → rebuilds B, prunes A

Steady state is apt-get update && apt-get install git on every run and a cache hit never. That alone is a modest cost. What makes it worth changing is that item 1 of this same PR turns that build's failure from a warning into exit 1 — so the prune raises the frequency of exactly the build whose failure now reds CI and blocks the deploy. The two halves of the PR pull against each other, and the description reasons about each in isolation.

The issue asked for "prune the other tags with that prefix", so this is faithful to it; the interaction is what the issue did not anticipate. A cheap fix keeps both properties: carry {{.CreatedAt}} (or {{.CreatedSince}}) in the format string and skip anything young, or keep the N newest. An image superseded an hour ago is not the dead weight the comment is about — one superseded a week ago is.

2. `DENO_GIT=optional` still dies on a missing `sha256sum`

Verified: DENO_GIT=optional PATH=<no sha256sum> sh .forgejo/deno.sh test … exits 1. The guard sits above the policy and the policy is only consulted on build failure, so a caller that has explicitly declared it needs no git — and therefore no derived image at all — is refused over a hash tool it never uses. The correct degradation for that caller is the one the fallback already implements: warn and run $IMAGE.

"There is no second digest to use" is the right answer for a caller that needs the derived image. It is not the right answer for one that has said it doesn't. Both hatches exist; they should compose.

3. The verification is not in the tree

Nineteen assertions against a stubbed docker CLI, and none of them are in the diff. The next person to touch this script has the same nothing that made these three follow-ups necessary.

The invariant that will actually rot is not the shell — it is the YAML. tools/ci_pins_test.ts already exists to stop CI config from rotting, runs under plain deno test --allow-read (note deno task test grants only --allow-run=git, so a real shell harness would need a task change; a text assertion needs nothing), and this is a fifteen-line third case: every step in deps.yml whose run invokes deno.sh carries DENO_GIT: optional, and build.yml's does not. Add a fourth step to deps.yml without the env and nobody finds out until the day the Debian mirror is down and the workflow this PR set out to protect goes red anyway.

Smaller

*"<none>"* is close to dead code. docker images opsdeck-deno-ci filters by repository, and an image orphaned by a rebuild loses its repository too — it lists as <none>:<none> and the filter excludes it. The only way .Tag is <none> under that filter is a digest-only reference, which this script never creates. Harmless, and I would keep it, but the table's "<none> untouched" row asserts against stub output rather than against anything docker produces.
Nothing prunes the build cache. docker rmi untags the derived image; the apt-get layer each docker build writes stays in the builder cache, and finding 1 makes that cache grow faster. Probably out of scope — but docs/dependencies.md now says "It prunes the superseded tags after a successful build", which the next person reading it for a disk problem will take as "disk is handled".
Docs, precise: the tag is the first 16 hex of a sha256sum, not "a sha256sum of the whole recipe"; and deps.yml sets DENO_GIT on every step that calls deno.sh — Checkout doesn't.

On the option not taken: rejecting the issue's first choice because "one pass over deno.sh closes all three" is the weakest paragraph in the description. The issue's framing was about which file the fix lands in, not about which fix is correct, and ignore: !hasGit in commit_test.ts is untouched — so the guarantee this PR buys is "CI refuses to run without git", not "the suite cannot silently skip". That is the right guarantee for CI and I am not asking for the other one. But 2 and 3 above are the two places where "the wrapper alone enforces it" leaks, and both are cheap to close.

I re-ran the verification independently — my own stub `docker` on `PATH`, a throwaway `docker/Dockerfile` carrying the current pin, and the script exactly as it stands at 72641eb — rather than taking the table in the description on trust. Every one of its eight rows reproduces, so the mechanics are right. What I want changed is where two of the three items interact, and one place where the two escape hatches don't compose. ## What holds up - **The tag does not move.** `sha256sum | cut -c1-16` and main's `sha256sum 2>/dev/null || cksum` → `tr -cd '[:alnum:]'` → `cut -c1-16` both produce `f4224312806e885c` for the current recipe. Dropping the `tr` is safe — `cut -c1-16` never reaches the ` -` suffix — and it matters that it is safe: every runner that already holds the derived image keeps its cache across this merge. Worth having checked, because a silent tag change here would have been a fleet-wide rebuild disguised as a cleanup. - **The prune is correctly scoped and correctly cowardly.** Cold build with two stale tags: both `rmi`'d, the tag just built skipped, `denoland/deno` never a candidate. `rmi` returning non-zero does not stop the run. Cached path does no build and no prune. - **`set -e` does not fire on the prune pipeline.** The `while` exits 0 whether the last body execution was `continue` or `docker rmi … || true`, so a run whose newest listed tag happens to be `$CI_IMAGE` does not die on the loop's status. That is load-bearing and easy to get wrong; it is right here. - **`case $stale in "$CI_IMAGE" | …`** — `$CI_IMAGE` is quoted so the tag is a literal, and the `case` word needs no quoting. Fine. - **build.yml deliberately left at the default.** `deno task ci` is the caller that runs the git-guarded suites, and it is the one that now fails hard. That is the whole point of the change and it is wired correctly. ## 1. The prune and the hard failure compound into cache thrash Both workflows are `runs-on: cth-ubuntu-latest`, and the recipe is a function of the Dockerfile's `FROM`. So the moment two branches disagree on the Deno pin — which is precisely what a dep-check bump PR is, and filing those is that tool's entire job — the shared runner alternates: - PR push → `verify` builds recipe B, prunes A - main push → `verify` misses A (just pruned), rebuilds A, prunes B - next PR push → rebuilds B, prunes A Steady state is `apt-get update && apt-get install git` on every run and a cache hit never. That alone is a modest cost. What makes it worth changing is that item 1 of this same PR turns that build's failure from a warning into `exit 1` — so the prune raises the frequency of exactly the build whose failure now reds CI and blocks the deploy. The two halves of the PR pull against each other, and the description reasons about each in isolation. The issue asked for "prune the other tags with that prefix", so this is faithful to it; the interaction is what the issue did not anticipate. A cheap fix keeps both properties: carry `{{.CreatedAt}}` (or `{{.CreatedSince}}`) in the format string and skip anything young, or keep the N newest. An image superseded an hour ago is not the dead weight the comment is about — one superseded a week ago is. ## 2. `DENO_GIT=optional` still dies on a missing `sha256sum` Verified: `DENO_GIT=optional PATH=<no sha256sum> sh .forgejo/deno.sh test …` exits 1. The guard sits above the policy and the policy is only consulted on build failure, so a caller that has explicitly declared it needs no git — and therefore no derived image at all — is refused over a hash tool it never uses. The correct degradation for that caller is the one the fallback already implements: warn and run `$IMAGE`. "There is no second digest to use" is the right answer for a caller that needs the derived image. It is not the right answer for one that has said it doesn't. Both hatches exist; they should compose. ## 3. The verification is not in the tree Nineteen assertions against a stubbed docker CLI, and none of them are in the diff. The next person to touch this script has the same nothing that made these three follow-ups necessary. The invariant that will actually rot is not the shell — it is the YAML. `tools/ci_pins_test.ts` already exists to stop CI config from rotting, runs under plain `deno test --allow-read` (note `deno task test` grants only `--allow-run=git`, so a real shell harness would need a task change; a text assertion needs nothing), and this is a fifteen-line third case: every step in `deps.yml` whose `run` invokes `deno.sh` carries `DENO_GIT: optional`, and `build.yml`'s does not. Add a fourth step to `deps.yml` without the env and nobody finds out until the day the Debian mirror is down and the workflow this PR set out to protect goes red anyway. ## Smaller - **`*"<none>"*` is close to dead code.** `docker images opsdeck-deno-ci` filters by repository, and an image orphaned by a rebuild loses its repository too — it lists as `<none>:<none>` and the filter excludes it. The only way `.Tag` is `<none>` under that filter is a digest-only reference, which this script never creates. Harmless, and I would keep it, but the table's "`<none>` untouched" row asserts against stub output rather than against anything docker produces. - **Nothing prunes the build cache.** `docker rmi` untags the derived image; the `apt-get` layer each `docker build` writes stays in the builder cache, and finding 1 makes that cache grow faster. Probably out of scope — but `docs/dependencies.md` now says "It prunes the superseded tags after a successful build", which the next person reading it for a disk problem will take as "disk is handled". - **Docs, precise:** the tag is the first 16 hex of a `sha256sum`, not "a `sha256sum` of the whole recipe"; and `deps.yml` sets `DENO_GIT` on every step *that calls `deno.sh`* — Checkout doesn't. On the option not taken: rejecting the issue's first choice because "one pass over `deno.sh` closes all three" is the weakest paragraph in the description. The issue's framing was about which file the fix lands in, not about which fix is correct, and `ignore: !hasGit` in `commit_test.ts` is untouched — so the guarantee this PR buys is "CI refuses to run without git", not "the suite cannot silently skip". That is the right guarantee for CI and I am not asking for the other one. But 2 and 3 above are the two places where "the wrapper alone enforces it" leaks, and both are cheap to close.

.forgejo/deno.sh Outdated

					
				@ -66,0 +67,4 @@

				# computed it — a different value for the same recipe, so a mixed fleet built

				# the same image twice and cached neither for the other. (`cksum` is a CRC32

				# besides, where everything around it reasons about a hash.)

				if ! command -v sha256sum >/dev/null 2>&1; then

This guard runs before GIT_POLICY is read, and the policy is only consulted on build failure — so a caller that has declared it needs no git, and therefore needs no derived image at all, still exits 1 here.

Verified: DENO_GIT=optional PATH=<no sha256sum> sh .forgejo/deno.sh test x → rc=1.

"There is no second digest to use" is correct for a caller that needs the image. For one that has said it doesn't, the right degradation is the one thirty lines down: warn and run $IMAGE. Reading GIT_POLICY above this block and branching on it costs two lines.

This guard runs before `GIT_POLICY` is read, and the policy is only consulted on build failure — so a caller that has declared it needs no git, and therefore needs no derived image at all, still exits 1 here. Verified: `DENO_GIT=optional PATH=<no sha256sum> sh .forgejo/deno.sh test x` → rc=1. "There is no second digest to use" is correct for a caller that needs the image. For one that has said it doesn't, the right degradation is the one thirty lines down: warn and run `$IMAGE`. Reading `GIT_POLICY` above this block and branching on it costs two lines.

.forgejo/deno.sh Outdated

					
				@ -74,0 +96,4 @@

				    # one per recipe or toolchain change — each a full Deno image. Best

				    # effort: another job may be running one of these, and losing that race

				    # is not this run's problem.

				    docker images --format '{{.Repository}}:{{.Tag}}' opsdeck-deno-ci \

This prunes every other recipe on the runner, and both workflows share cth-ubuntu-latest. As soon as two branches disagree on the Dockerfile pin — a dep-check bump PR against main is exactly that — the two recipes evict each other in a loop: PR builds B and prunes A, main rebuilds A and prunes B, next PR push rebuilds B. Cache hit never.

The cost is one apt-get install git per run, which on its own would be a shrug. It matters because item 1 of this PR makes that build's failure fatal, so the prune raises the rate of exactly the build that now reds CI and blocks the deploy.

--format '{{.Repository}}:{{.Tag}} {{.CreatedAt}}' and skipping anything young keeps the stated goal (a long-lived runner does not hoard a full Deno image per recipe change) without evicting a sibling branch's image mid-flight.

This prunes every other recipe on the runner, and both workflows share `cth-ubuntu-latest`. As soon as two branches disagree on the Dockerfile pin — a dep-check bump PR against main is exactly that — the two recipes evict each other in a loop: PR builds B and prunes A, main rebuilds A and prunes B, next PR push rebuilds B. Cache hit never. The cost is one `apt-get install git` per run, which on its own would be a shrug. It matters because item 1 of this PR makes that build's failure fatal, so the prune raises the rate of exactly the build that now reds CI and blocks the deploy. `--format '{{.Repository}}:{{.Tag}} {{.CreatedAt}}'` and skipping anything young keeps the stated goal (a long-lived runner does not hoard a full Deno image per recipe change) without evicting a sibling branch's image mid-flight.

.forgejo/deno.sh Outdated

					
				@ -74,0 +99,4 @@

				    docker images --format '{{.Repository}}:{{.Tag}}' opsdeck-deno-ci \

				      2>/dev/null | while read -r stale; do

				      case $stale in

				      "$CI_IMAGE" | *"<none>"*) continue ;;

docker images opsdeck-deno-ci filters by repository, and an image orphaned by a rebuild loses its repository along with its tag — it lists as <none>:<none> and never reaches this loop. The only way .Tag is <none> under a repository filter is a digest-only reference, which nothing here creates.

Keep it, it costs nothing. But the description's "<none> untouched" row is asserting against the stub's output rather than against anything the real CLI emits under this exact invocation — worth knowing when the table is read as coverage.

`docker images opsdeck-deno-ci` filters by repository, and an image orphaned by a rebuild loses its repository along with its tag — it lists as `<none>:<none>` and never reaches this loop. The only way `.Tag` is `<none>` under a repository filter is a digest-only reference, which nothing here creates. Keep it, it costs nothing. But the description's "`<none>` untouched" row is asserting against the stub's output rather than against anything the real CLI emits under this exact invocation — worth knowing when the table is read as coverage.

.forgejo/workflows/deps.yml

					
				@ -50,1 +55,4 @@

				      # unreachable Debian mirror should not take this workflow down with it.

				      - name: Test

				        env:

				          DENO_GIT: optional

This is the part that will rot. A fourth step added here without DENO_GIT: optional behaves identically until the day the Debian mirror is unreachable, and then takes down the workflow this PR set out to keep up — the failure mode is invisible right up to the point where it matters.

tools/ci_pins_test.ts is already the place where "CI config must not drift" lives, and it runs on plain --allow-read, so this is a text assertion and not a harness: every step in deps.yml whose run contains deno.sh carries the env, and build.yml's does not. That second half is worth asserting too — it is the whole point of the change and nothing currently stops someone from "fixing" a red main by pasting the env there.

This is the part that will rot. A fourth step added here without `DENO_GIT: optional` behaves identically until the day the Debian mirror is unreachable, and then takes down the workflow this PR set out to keep up — the failure mode is invisible right up to the point where it matters. `tools/ci_pins_test.ts` is already the place where "CI config must not drift" lives, and it runs on plain `--allow-read`, so this is a text assertion and not a harness: every step in `deps.yml` whose `run` contains `deno.sh` carries the env, and `build.yml`'s does not. That second half is worth asserting too — it is the whole point of the change and nothing currently stops someone from "fixing" a red main by pasting the env there.

julian added 1 commit

2026-08-16 21:59:05 +02:00

ci(deno.sh): age-gate the prune, compose the hatches, pin the YAML wiring

Build and Deploy / verify (pull_request) Successful in 1m30s

Details

Build and Deploy / build (pull_request) Has been skipped

Details

Dependency Check / dependencies (pull_request) Successful in 1m6s

Details

f492244369

Prune only images at least two days old. Both workflows share one
runner, so two branches that disagree on the Dockerfile pin - a
dep-check bump PR against main is exactly that - would evict each
other's recipe on every push: rebuild and eviction alternating, a cache
hit never, and every rebuild a fresh chance to fail a run that needs
git. `CreatedSince` counts in hours until 48, so the larger humanized
units are the gate.

Read GIT_POLICY before the sha256sum guard. A caller that declared
DENO_GIT=optional needs no derived image at all, so a missing digest
tool degrades for it the way a failed build already did: warn and run
the base image. Only a caller that needs git still exits 1 there. The
build/prune block is skipped when a hatch already fell back to the base
image, so nothing ever builds the recipe under the base image's name.

Assert the DENO_GIT wiring in tools/ci_pins_test.ts: every deps.yml
step whose run: enters deno.sh carries the env, and no build.yml step
sets it at all - the second half so a red main cannot be "fixed" by
pasting the opt-out into the workflow whose hard failure is the point.
The check reads from the run: key onward, because the surrounding
comments name the script too.

docs/dependencies.md: the tag is the first 16 hex of the sha256sum, the
prune is age-gated and untags only (builder cache stays), and deps.yml
sets the env on the steps that call the wrapper, not literally all.

Verified against the stubbed docker CLI: 30 assertions across cached /
build-ok / build-fail x policy / rmi-fail / no-sha256sum x policy;
young and <none> tags survive the prune, the tag stays
f4224312806e885c, and the new test goes red on either invariant
mutation. Repository gates: deno fmt --check, deno lint, deno task
check, sh -n, tools/dep-check (55 passed), packages/server/tests (109
passed; the one failure, an interval leak in external_modules_test.ts,
reproduces on a clean HEAD and is unrelated).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

julian commented

2026-08-16 21:59:26 +02:00

All three requested changes are in f492244, plus the smaller doc corrections.

1. Prune is age-gated. The format string now carries {{.CreatedSince}}, and only tags whose age is in days/weeks/months/years go — docker humanizes in hours until 48, so the gate is effectively "at least two days old". A sibling branch's recipe on the shared runner is hours old at the point it matters and survives; the week-old leftovers the comment was about still go. The comment in the script now names the interaction you described (PR/main alternating eviction, made worse by the now-fatal rebuild) rather than reasoning about the prune in isolation.

2. The hatches compose. GIT_POLICY is read before the sha256sum guard. A caller that declared DENO_GIT=optional needs no derived image at all, so a missing sha256sum degrades for it exactly the way a failed build already did: warn and run $IMAGE. A caller that needs git still exits 1 with the original message. The build/prune block is additionally skipped whenever a hatch has already fallen back (CI_IMAGE = $IMAGE), so the missing-tool path can never docker build the recipe under the base image's name.

3. The YAML invariant is in the tree. tools/ci_pins_test.ts gains a third test: every deps.yml step whose run: invokes deno.sh carries DENO_GIT: optional, and no build.yml step sets DENO_GIT at all — the second half so a red main cannot be "fixed" by pasting the opt-out into the workflow whose hard failure is the point. It reads from the run: key onward because the surrounding comments name the script too, and it matches DENO_GIT: as a YAML line so the comment in deps.yml (which says DENO_GIT=optional) is not a false positive. Mutation-tested both ways: removing one env entry from deps.yml and adding one to build.yml each turn the test red, naming the step.

Smaller:

<none> guard kept as-is, per your note. Agreed it is stub-shaped rather than CLI-shaped under a repository filter; it stays as a cheap guard, and I have stopped presenting that row as coverage.
docs/dependencies.md now says the prune is age-gated and untags only — the layers each docker build wrote stay in the builder cache, so it "keeps the image list short, not the disk clean". Nobody chasing a disk problem should read "prunes" as "disk is handled".
Docs also corrected: the tag is the first 16 hex characters of the sha256sum (not the whole digest), and deps.yml sets the env on every step that calls the wrapper.

Re-verification. The stub harness is now 30 assertions: cached / build-ok / build-fail × policy / rmi-fail / no-sha256sum × policy. New rows: a young ("2 hours ago") tag survives the prune while day- and week-aged tags go; the no-sha256sum + optional run warns, never calls docker build or docker image inspect, and creates the container from the base image. The missing-sha256sum branches ran against a PATH genuinely lacking it. Tag unchanged at f4224312806e885c, so no fleet rebuild rides along. Gates: deno fmt --check, deno lint, deno task check, sh -n, tools/dep-check (55 passed), packages/server/tests (109 passed — the one failure is an interval leak in external_modules_test.ts that reproduces on a clean HEAD, unrelated to this diff).

All three requested changes are in f492244, plus the smaller doc corrections. **1. Prune is age-gated.** The format string now carries `{{.CreatedSince}}`, and only tags whose age is in days/weeks/months/years go — docker humanizes in hours until 48, so the gate is effectively "at least two days old". A sibling branch's recipe on the shared runner is hours old at the point it matters and survives; the week-old leftovers the comment was about still go. The comment in the script now names the interaction you described (PR/main alternating eviction, made worse by the now-fatal rebuild) rather than reasoning about the prune in isolation. **2. The hatches compose.** `GIT_POLICY` is read before the sha256sum guard. A caller that declared `DENO_GIT=optional` needs no derived image at all, so a missing `sha256sum` degrades for it exactly the way a failed build already did: warn and run `$IMAGE`. A caller that needs git still exits 1 with the original message. The build/prune block is additionally skipped whenever a hatch has already fallen back (`CI_IMAGE` = `$IMAGE`), so the missing-tool path can never `docker build` the recipe under the base image's name. **3. The YAML invariant is in the tree.** `tools/ci_pins_test.ts` gains a third test: every `deps.yml` step whose `run:` invokes `deno.sh` carries `DENO_GIT: optional`, and no `build.yml` step sets `DENO_GIT` at all — the second half so a red main cannot be "fixed" by pasting the opt-out into the workflow whose hard failure is the point. It reads from the `run:` key onward because the surrounding comments name the script too, and it matches `DENO_GIT:` as a YAML line so the comment in deps.yml (which says `DENO_GIT=optional`) is not a false positive. Mutation-tested both ways: removing one env entry from deps.yml and adding one to build.yml each turn the test red, naming the step. **Smaller:** - `<none>` guard kept as-is, per your note. Agreed it is stub-shaped rather than CLI-shaped under a repository filter; it stays as a cheap guard, and I have stopped presenting that row as coverage. - `docs/dependencies.md` now says the prune is age-gated and *untags only* — the layers each `docker build` wrote stay in the builder cache, so it "keeps the image list short, not the disk clean". Nobody chasing a disk problem should read "prunes" as "disk is handled". - Docs also corrected: the tag is the first 16 hex characters of the `sha256sum` (not the whole digest), and `deps.yml` sets the env on every step *that calls the wrapper*. **Re-verification.** The stub harness is now 30 assertions: cached / build-ok / build-fail × policy / rmi-fail / no-`sha256sum` × policy. New rows: a young ("2 hours ago") tag survives the prune while day- and week-aged tags go; the no-`sha256sum` + `optional` run warns, never calls `docker build` or `docker image inspect`, and creates the container from the base image. The missing-`sha256sum` branches ran against a `PATH` genuinely lacking it. Tag unchanged at `f4224312806e885c`, so no fleet rebuild rides along. Gates: `deno fmt --check`, `deno lint`, `deno task check`, `sh -n`, `tools/dep-check` (55 passed), `packages/server/tests` (109 passed — the one failure is an interval leak in `external_modules_test.ts` that reproduces on a clean HEAD, unrelated to this diff).

julian requested review from thisilike

2026-08-16 21:59:29 +02:00

thisilike requested changes

2026-08-16 22:33:26 +02:00

Dismissed

thisilike left a comment

Re-ran everything from scratch at f492244 — my own stub docker on PATH rather than the PR's harness, plus a real daemon (docker 29.7.1, BuildKit) for the two questions a stub cannot answer. All three items do what the description says, and the mechanics reproduce. Two claims do not survive: the age gate does not protect a recipe that is in use, and the build.yml half of the new test does not hold.

What reproduces

The tag still does not move. The new sha256sum | cut -c1-16 and main's sha256sum || cksum → tr → cut both give tf4224312806e885c for the current recipe, recomputed here. No fleet-wide rebuild rides along.
The hatches compose. No sha256sum on PATH + DENO_GIT=optional: warns, and the whole docker call log is create -w /w denoland/deno:2.9.5@sha256:b429777… deno test … — no image inspect, no build, exactly as claimed. Same PATH without the opt-out: rc=1 with the "no second digest" message. Build-fail × policy behaves the same in both directions.
The age gate matches docker's real vocabulary. Fed Less than a second ago, About a minute ago, 44 minutes ago, About an hour ago, 2 hours ago, 47 hours ago, 2 days, 13 days, 3 weeks, 5 months, 2 years, <none>: exactly the day/week/month/year rows are rmi'd; the tag just built and <none> are skipped. 31 hours ago appears in my own daemon's image list, so the 48-hour boundary is not theoretical.
set -e still does not fire on the prune. rc=0 and the container is created for: an all-young list (every iteration ends in continue), a list holding only $CI_IMAGE, an empty list, docker images exiting non-zero, and rmi failing on every tag.
The new test catches your two mutations — env removed from a deps.yml step, env added to a build.yml step — and a third: a fourth deps.yml step that calls the wrapper without it.
deps.yml's opt-out is safe for a stronger reason than the comment gives. None of the three commands pass --allow-run, so they cannot spawn git at all. That is a firmer guarantee than "nothing there touches git", and it is the sentence I would put in the comment.
Gates re-run here: deno fmt --check (314 files), deno lint (196), deno task check, tools/dep-check (55 passed), sh -n.

1. The gate protects the recipe first built recently, not the one in use

Created does not reset when an untagged image is rebuilt from cache. On a real daemon:

first build:              rigtest-age:a | 1 second ago  | fb3a9f6407ca
rmi, sleep 3, rebuild:    rigtest-age:a | 4 seconds ago | fb3a9f6407ca

Same image ID, Created untouched — the rebuild is a cache hit and docker hands back the cached image, timestamp and all. (Metadata.LastTagTime does move on that rebuild: 22:29:16 → 22:29:20.)

So a recipe a branch has been using every day for a week reads as a week old, and the next prune to come past evicts it — the exact case the gate was added for, since a sibling branch's pin is precisely a recipe that is old and in use. And it does not self-correct: after the eviction the rebuild comes back still stamped a week old, so it is prunable again immediately. As long as the builder cache holds the layers that rebuild is seconds and no network — but the builder cache is the one thing nothing here prunes, as the docs now say, and once a runner reclaims it that rebuild is the real apt-get one whose failure this same PR made fatal.

{{.CreatedSince}} cannot express "last used". docker image inspect -f '{{.Metadata.LastTagTime}}' <tag> can: it means "when this runner last built or tagged this recipe", it moves on a cache-hit rebuild, and it converges — an evicted sibling that gets rebuilt is young again and stops being a target. One inspect per candidate, and the candidate list is short. Worth one caveat line: under the containerd image store that field can come back empty, so treat empty as "keep" rather than "prune".

If you would rather keep CreatedSince, then docs/dependencies.md's "a sibling branch on a different pin holds a recipe that is in use, not dead" is the sentence to correct — as written the gate does not deliver it.

2. `build.yml` can still opt out — three ways, one of them the documented spelling

Mutation-tested the second half. Caught: env: / DENO_GIT: optional on the step. Not caught, test stays green:

job-level env: under verify:
workflow-level env: at the top of build.yml
run: DENO_GIT=optional ./.forgejo/deno.sh task ci

All three opt out for real at runtime. The third is the form deno.sh's own comment documents (DENO_GIT=optional sh .forgejo/deno.sh test …), so the guard misses the spelling a reader of the script is likeliest to copy — and "reviving a red main" is exactly the moment someone copies from the error message and the script. docs/dependencies.md states "asserts both halves — … and no step in build.yml does", which is true of steps and not of the file.

The fix is smaller than what is there: for build.yml the invariant is not per-step at all, so assert on the whole text — assertEquals(Deno.readTextFileSync(BUILD).includes("DENO_GIT"), false, …). No step splitting, no env-shape assumption, all three closed, and build.yml names DENO_GIT nowhere today.

Smaller

The deps.yml half is shape-sensitive in both directions. All of these are false reds, which is the safe direction, but they are surprising: DENO_GIT: "optional" (same YAML, quoted) fails; the inline run: DENO_GIT=optional sh .forgejo/deno.sh … form fails; and a step that does not call the wrapper goes red when a comment naming deno.sh sits between its run: and the next step — reading from run: onward only handles comments above it, and the comment block in deps.yml today lands in the Checkout step's chunk, which passes only because Checkout has no run: at all. /DENO_GIT[:=]\s*"?optional/ per step covers the first two.
GIT_POLICY=${DENO_GIT:-required} reads every unrecognized value as required — DENO_GIT=yes, true, Optional all mean "git required". Safe direction, keep it; the test pinning the one working spelling in deps.yml is what makes it fine.
I could not check the "109 passed, interval leak in external_modules_test.ts" line: in my environment two of those cases fail on gpg signing timeouts from my own git config, so my run says nothing either way. The diff touches no server code, so nothing in that suite can be this PR's doing.

Close 2, and fix or re-word 1, and this is done.

Re-ran everything from scratch at `f492244` — my own stub `docker` on `PATH` rather than the PR's harness, plus a real daemon (docker 29.7.1, BuildKit) for the two questions a stub cannot answer. All three items do what the description says, and the mechanics reproduce. Two claims do not survive: the age gate does not protect a recipe that is in use, and the `build.yml` half of the new test does not hold. ## What reproduces - **The tag still does not move.** The new `sha256sum | cut -c1-16` and main's `sha256sum || cksum` → `tr` → `cut` both give `tf4224312806e885c` for the current recipe, recomputed here. No fleet-wide rebuild rides along. - **The hatches compose.** No `sha256sum` on `PATH` + `DENO_GIT=optional`: warns, and the whole docker call log is `create -w /w denoland/deno:2.9.5@sha256:b429777… deno test …` — no `image inspect`, no `build`, exactly as claimed. Same PATH without the opt-out: rc=1 with the "no second digest" message. Build-fail × policy behaves the same in both directions. - **The age gate matches docker's real vocabulary.** Fed `Less than a second ago`, `About a minute ago`, `44 minutes ago`, `About an hour ago`, `2 hours ago`, `47 hours ago`, `2 days`, `13 days`, `3 weeks`, `5 months`, `2 years`, `<none>`: exactly the day/week/month/year rows are `rmi`'d; the tag just built and `<none>` are skipped. `31 hours ago` appears in my own daemon's image list, so the 48-hour boundary is not theoretical. - **`set -e` still does not fire on the prune.** rc=0 and the container is created for: an all-young list (every iteration ends in `continue`), a list holding only `$CI_IMAGE`, an empty list, `docker images` exiting non-zero, and `rmi` failing on every tag. - **The new test catches your two mutations** — env removed from a `deps.yml` step, env added to a `build.yml` step — and a third: a fourth `deps.yml` step that calls the wrapper without it. - **`deps.yml`'s opt-out is safe for a stronger reason than the comment gives.** None of the three commands pass `--allow-run`, so they cannot spawn git at all. That is a firmer guarantee than "nothing there touches git", and it is the sentence I would put in the comment. - Gates re-run here: `deno fmt --check` (314 files), `deno lint` (196), `deno task check`, `tools/dep-check` (55 passed), `sh -n`. ## 1. The gate protects the recipe first built recently, not the one in use `Created` does not reset when an untagged image is rebuilt from cache. On a real daemon: ``` first build: rigtest-age:a | 1 second ago | fb3a9f6407ca rmi, sleep 3, rebuild: rigtest-age:a | 4 seconds ago | fb3a9f6407ca ``` Same image ID, `Created` untouched — the rebuild is a cache hit and docker hands back the cached image, timestamp and all. (`Metadata.LastTagTime` *does* move on that rebuild: `22:29:16` → `22:29:20`.) So a recipe a branch has been using every day for a week reads as a week old, and the next prune to come past evicts it — the exact case the gate was added for, since a sibling branch's pin is precisely a recipe that is old and in use. And it does not self-correct: after the eviction the rebuild comes back still stamped a week old, so it is prunable again immediately. As long as the builder cache holds the layers that rebuild is seconds and no network — but the builder cache is the one thing nothing here prunes, as the docs now say, and once a runner reclaims it that rebuild is the real `apt-get` one whose failure this same PR made fatal. `{{.CreatedSince}}` cannot express "last used". `docker image inspect -f '{{.Metadata.LastTagTime}}' <tag>` can: it means "when this runner last built or tagged this recipe", it moves on a cache-hit rebuild, and it converges — an evicted sibling that gets rebuilt is young again and stops being a target. One inspect per candidate, and the candidate list is short. Worth one caveat line: under the containerd image store that field can come back empty, so treat empty as "keep" rather than "prune". If you would rather keep `CreatedSince`, then `docs/dependencies.md`'s "a sibling branch on a different pin holds a recipe that is in use, not dead" is the sentence to correct — as written the gate does not deliver it. ## 2. `build.yml` can still opt out — three ways, one of them the documented spelling Mutation-tested the second half. Caught: `env: / DENO_GIT: optional` on the step. **Not** caught, test stays green: - job-level `env:` under `verify:` - workflow-level `env:` at the top of `build.yml` - `run: DENO_GIT=optional ./.forgejo/deno.sh task ci` All three opt out for real at runtime. The third is the form `deno.sh`'s own comment documents (`DENO_GIT=optional sh .forgejo/deno.sh test …`), so the guard misses the spelling a reader of the script is likeliest to copy — and "reviving a red main" is exactly the moment someone copies from the error message and the script. `docs/dependencies.md` states "asserts both halves — … and no step in `build.yml` does", which is true of steps and not of the file. The fix is smaller than what is there: for `build.yml` the invariant is not per-step at all, so assert on the whole text — `assertEquals(Deno.readTextFileSync(BUILD).includes("DENO_GIT"), false, …)`. No step splitting, no env-shape assumption, all three closed, and `build.yml` names `DENO_GIT` nowhere today. ## Smaller - The `deps.yml` half is shape-sensitive in both directions. All of these are false reds, which is the safe direction, but they are surprising: `DENO_GIT: "optional"` (same YAML, quoted) fails; the inline `run: DENO_GIT=optional sh .forgejo/deno.sh …` form fails; and a step that does *not* call the wrapper goes red when a comment naming `deno.sh` sits between its `run:` and the next step — reading from `run:` onward only handles comments *above* it, and the comment block in `deps.yml` today lands in the Checkout step's chunk, which passes only because Checkout has no `run:` at all. `/DENO_GIT[:=]\s*"?optional/` per step covers the first two. - `GIT_POLICY=${DENO_GIT:-required}` reads every unrecognized value as `required` — `DENO_GIT=yes`, `true`, `Optional` all mean "git required". Safe direction, keep it; the test pinning the one working spelling in `deps.yml` is what makes it fine. - I could not check the "109 passed, interval leak in `external_modules_test.ts`" line: in my environment two of those cases fail on gpg signing timeouts from my own git config, so my run says nothing either way. The diff touches no server code, so nothing in that suite can be this PR's doing. Close 2, and fix or re-word 1, and this is done.

.forgejo/deno.sh Outdated

					
				@ -74,0 +114,4 @@

				    # evicts the PR's, cache hit never — and a failed rebuild is fatal for

				    # every caller that needs git. `CreatedSince` is humanized and counts in

				    # hours until 48, so the larger units are the age gate.

				    docker images --format '{{.Repository}}:{{.Tag}} {{.CreatedSince}}' \

CreatedSince is the wrong clock for "in use". Measured on a real daemon: docker rmi the tag, rebuild the same recipe from cache, and you get the same image ID with the original Created — 1 second ago before, 4 seconds ago after a 3-second sleep, not reset.

So a sibling branch's recipe that has been in daily use for a week reads as a week old and gets evicted here, which is the case this gate was added for. And it does not recover: the rebuild comes back still stamped a week old, so it is prunable again on the next pass. Cheap while the builder cache holds the layers — that cache is the one thing nothing prunes — and a real apt-get build once it doesn't, which this PR made fatal.

docker image inspect -f '{{.Metadata.LastTagTime}}' <tag> is the signal that behaves: it moved (22:29:16 → 22:29:20) across exactly that untag/rebuild, so an evicted sibling becomes young again and stops being a target. Empty under the containerd image store, so treat empty as keep.

`CreatedSince` is the wrong clock for "in use". Measured on a real daemon: `docker rmi` the tag, rebuild the same recipe from cache, and you get the **same image ID with the original `Created`** — `1 second ago` before, `4 seconds ago` after a 3-second sleep, not reset. So a sibling branch's recipe that has been in daily use for a week reads as a week old and gets evicted here, which is the case this gate was added for. And it does not recover: the rebuild comes back still stamped a week old, so it is prunable again on the next pass. Cheap while the builder cache holds the layers — that cache is the one thing nothing prunes — and a real `apt-get` build once it doesn't, which this PR made fatal. `docker image inspect -f '{{.Metadata.LastTagTime}}' <tag>` is the signal that behaves: it moved (`22:29:16` → `22:29:20`) across exactly that untag/rebuild, so an evicted sibling becomes young again and stops being a target. Empty under the containerd image store, so treat empty as keep.

tools/ci_pins_test.ts Outdated

					
				@ -54,0 +72,4 @@

				  // files and talks to registries, and nothing there touches git.

				  const missing = steps(DEPS)

				    .filter((step) =>

				      runsDenoSh(step) && !/^\s*DENO_GIT:\s*optional$/m.test(step)

Shape-sensitive in ways that are all false reds — safe direction, worth knowing before someone hits one:

DENO_GIT: "optional" — same YAML, quoted — fails.
The inline run: DENO_GIT=optional sh .forgejo/deno.sh … form fails, though it works at runtime.
A step that does not call the wrapper goes red if a comment naming deno.sh sits between its run: and the next step. Reading from run: onward only handles comments above the key; today's comment block lands in the Checkout chunk and passes only because Checkout has no run: at all.

/DENO_GIT[:=]\s*"?optional/ covers the first two.

Shape-sensitive in ways that are all false reds — safe direction, worth knowing before someone hits one: - `DENO_GIT: "optional"` — same YAML, quoted — fails. - The inline `run: DENO_GIT=optional sh .forgejo/deno.sh …` form fails, though it works at runtime. - A step that does *not* call the wrapper goes red if a comment naming `deno.sh` sits between its `run:` and the next step. Reading from `run:` onward only handles comments *above* the key; today's comment block lands in the Checkout chunk and passes only because Checkout has no `run:` at all. `/DENO_GIT[:=]\s*"?optional/` covers the first two.

tools/ci_pins_test.ts Outdated

					
				@ -54,0 +85,4 @@

				  // git-guarded suites, and failing hard without git is the point — pasting

				  // the env here to revive a red main defeats the arrangement.

				  const optedOut = steps(BUILD)

				    .filter((step) => /^\s*DENO_GIT:/m.test(step))

This half does not hold. Mutation-tested — all three of these leave the test green and all three opt out for real:

job-level env: under verify:
workflow-level env: at the top of the file
run: DENO_GIT=optional ./.forgejo/deno.sh task ci

The last one is the spelling deno.sh's own comment documents, which is what someone reviving a red main would copy.

For build.yml the invariant is not per-step, so drop the step splitting here entirely:

assertEquals(
  Deno.readTextFileSync(BUILD).includes("DENO_GIT"),
  false,
  `${BUILD} opts out of the git requirement`,
);

Shorter than what is here, and it closes all three. build.yml names DENO_GIT nowhere today.

This half does not hold. Mutation-tested — all three of these leave the test green and all three opt out for real: - job-level `env:` under `verify:` - workflow-level `env:` at the top of the file - `run: DENO_GIT=optional ./.forgejo/deno.sh task ci` The last one is the spelling `deno.sh`'s own comment documents, which is what someone reviving a red main would copy. For `build.yml` the invariant is not per-step, so drop the step splitting here entirely: ```ts assertEquals( Deno.readTextFileSync(BUILD).includes("DENO_GIT"), false, `${BUILD} opts out of the git requirement`, ); ``` Shorter than what is here, and it closes all three. `build.yml` names `DENO_GIT` nowhere today.

julian referenced this pull request from a commit

2026-08-16 23:03:36 +02:00

ci(deno.sh): the prune gate asks when a tag was last needed, not first built

julian added 1 commit

2026-08-16 23:03:36 +02:00

ci(deno.sh): the prune gate asks when a tag was last needed, not first built

Build and Deploy / verify (pull_request) Successful in 1m19s

Details

Build and Deploy / build (pull_request) Has been skipped

Details

Dependency Check / dependencies (pull_request) Successful in 1m11s

Details

d372259192

Both blocking items from the review on #42:

- `Created` never moves on a cache-hit rebuild, so a sibling branch's
  in-use recipe read as old as its first build, was evicted on every
  pass, and came back from the rebuild still prunable — forever. The
  gate is now `LastTagTime` per candidate, which that rebuild does
  refresh, so an evicted recipe that comes back is young again and
  stops being a target. The containerd image store can answer with the
  zero time: not digits, so kept — a missing answer never prunes.

- build.yml's half of the DENO_GIT test asserted on step env blocks,
  and a job-level env, a workflow-level env and an inline
  DENO_GIT=optional on the run line all opt out at runtime without
  one. The invariant is the file: build.yml must not name DENO_GIT
  anywhere. The deps.yml half now accepts the spellings that opt out
  for real — the quoted value and the inline run form.

Smaller, same review: deps.yml's comment states the firmer guarantee —
none of its commands pass --allow-run, so they could not spawn git if
they tried — and docs/dependencies.md describes the gate the prune
actually has.

Verified against the stubbed docker CLI: 44 assertions across cached /
build-success with a mixed candidate list / build-fail x policy /
rmi-fail / images-fail / inspect-fail / no-sha256sum x policy, and 8
YAML mutations that each flip tools/ci_pins_test.ts in the demanded
direction. deno fmt --check, deno lint, deno task check, tools/ (58
passed), sh -n.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

julian commented

2026-08-16 23:03:55 +02:00

Both blocking items are in d372259, plus the smaller ones.

1. The gate is `LastTagTime` now

Per candidate: docker image inspect -f '{{.Metadata.LastTagTime.Unix}}', prune only past 48 h. Your rig demonstration is the whole argument — Created survives a cache-hit rebuild, so the in-use sibling recipe read as old forever and the gate never delivered the sentence the docs claimed — so I took the fix, not the reword. LastTagTime moves on that rebuild, so an evicted recipe that comes back is young again and converges out of the target set.

The caveat is honored in both shapes it can arrive in: the zero time prints as a negative number through .Unix, and an empty answer is empty — the guard is case $last in '' | *[!0-9]*) continue, so anything that is not a plain digit string is kept. A failed inspect (candidate untagged mid-loop) is || continue, also kept. The set -e property you checked still holds: every body path ends in continue or || true, and the loop exits 0 for an all-young list, an empty list, docker images failing, inspect failing on every candidate, and rmi failing on every candidate — all re-asserted in the harness.

docs/dependencies.md now describes the gate the prune actually has: "neither built nor tagged for two days", with the Created-vs-LastTagTime reason attached.

2. `build.yml` cannot opt out in any spelling

The per-step check is gone; the assertion is on the file:

assertEquals(Deno.readTextFileSync(BUILD).includes("DENO_GIT"), false, …)

Your three green mutations — job-level env:, workflow-level env:, inline DENO_GIT=optional on the run line — are all red now, alongside the step-level one. The deps.yml half takes your regex, widened one character for single quotes: /DENO_GIT[:=]\s*["']?optional/ per step that runs deno.sh, so the quoted value and the inline run form no longer false-red. The comment-between-steps false red stays as is — safe direction, and the step chunking that causes it is what keeps the check readable.

Smaller

deps.yml's comment (and the docs) now state your firmer guarantee: none of the three commands pass --allow-run, so they could not spawn git if they tried.
GIT_POLICY reading every unrecognized value as required stays, per your note — the test pinning the working spelling is what makes it fine.

Verification

Same stub-docker harness shape as before, rebuilt for the new gate: 44 assertions across cached / build-success against a mixed candidate list (stale pruned, young kept, $CI_IMAGE and <none> never inspected, empty and zero-time and inspect-failure kept) / build-fail × both policies / rmi fail / docker images fail / no-sha256sum × both policies. The 8 YAML mutations above each flip tools/ci_pins_test.ts in the demanded direction. Gates: deno fmt --check (314), deno lint (196), deno task check, tools/ (58 passed), sh -n.

What the stub still cannot answer: whether {{.Metadata.LastTagTime.Unix}} renders on the runner's actual docker version — the template method call is standard Go text/template over a time.Time, but the first real evaluation happens in CI here.

Both blocking items are in `d372259`, plus the smaller ones. ## 1. The gate is `LastTagTime` now Per candidate: `docker image inspect -f '{{.Metadata.LastTagTime.Unix}}'`, prune only past 48 h. Your rig demonstration is the whole argument — `Created` survives a cache-hit rebuild, so the in-use sibling recipe read as old forever and the gate never delivered the sentence the docs claimed — so I took the fix, not the reword. `LastTagTime` moves on that rebuild, so an evicted recipe that comes back is young again and converges out of the target set. The caveat is honored in both shapes it can arrive in: the zero time prints as a negative number through `.Unix`, and an empty answer is empty — the guard is `case $last in '' | *[!0-9]*) continue`, so anything that is not a plain digit string is kept. A failed `inspect` (candidate untagged mid-loop) is `|| continue`, also kept. The `set -e` property you checked still holds: every body path ends in `continue` or `|| true`, and the loop exits 0 for an all-young list, an empty list, `docker images` failing, `inspect` failing on every candidate, and `rmi` failing on every candidate — all re-asserted in the harness. `docs/dependencies.md` now describes the gate the prune actually has: "neither built nor tagged for two days", with the `Created`-vs-`LastTagTime` reason attached. ## 2. `build.yml` cannot opt out in any spelling The per-step check is gone; the assertion is on the file: ```ts assertEquals(Deno.readTextFileSync(BUILD).includes("DENO_GIT"), false, …) ``` Your three green mutations — job-level `env:`, workflow-level `env:`, inline `DENO_GIT=optional` on the run line — are all red now, alongside the step-level one. The deps.yml half takes your regex, widened one character for single quotes: `/DENO_GIT[:=]\s*["']?optional/` per step that runs `deno.sh`, so the quoted value and the inline run form no longer false-red. The comment-between-steps false red stays as is — safe direction, and the step chunking that causes it is what keeps the check readable. ## Smaller - deps.yml's comment (and the docs) now state your firmer guarantee: none of the three commands pass `--allow-run`, so they could not spawn git if they tried. - `GIT_POLICY` reading every unrecognized value as `required` stays, per your note — the test pinning the working spelling is what makes it fine. ## Verification Same stub-docker harness shape as before, rebuilt for the new gate: 44 assertions across cached / build-success against a mixed candidate list (stale pruned, young kept, `$CI_IMAGE` and `<none>` never inspected, empty and zero-time and inspect-failure kept) / build-fail × both policies / `rmi` fail / `docker images` fail / no-`sha256sum` × both policies. The 8 YAML mutations above each flip `tools/ci_pins_test.ts` in the demanded direction. Gates: `deno fmt --check` (314), `deno lint` (196), `deno task check`, `tools/` (58 passed), `sh -n`. What the stub still cannot answer: whether `{{.Metadata.LastTagTime.Unix}}` renders on the runner's actual docker version — the template method call is standard Go text/template over a `time.Time`, but the first real evaluation happens in CI here.

julian requested review from thisilike

2026-08-16 23:03:58 +02:00

thisilike requested changes

2026-08-21 16:56:15 +02:00

Dismissed

thisilike left a comment

Third pass at d372259. I did not reuse your harness or mine: this round is a real daemon (docker 29.7.2), the actual .forgejo/deno.sh from this commit driving real docker build / docker images / docker image inspect / docker rmi, and 14 mutations against tools/ci_pins_test.ts rather than your 8. The two items you took are the right fixes and the mechanics reproduce. Item 1 is closer but still does not deliver the sentence the docs claim, and the deps.yml half of the new test can be satisfied by prose.

What reproduces, including the three things nobody had run for real

The cold path works end-to-end. This is the one you flagged as unverified. Real daemon, real recipe, real docker cp and docker create: the derived image builds, the command runs, and git --version inside it answers git version 2.47.3. No longer a CI-first.
{{.Metadata.LastTagTime.Unix}} renders. Your open question, answered: 1787323657 on 29.7.2. And a bad template field exits 1, so the || continue genuinely keeps rather than silently reading an empty string.
Created vs LastTagTime is exactly as you argued. Build, rmi, sleep 3, rebuild from layer cache: same image ID, Created=…16:47:35.985850282 unchanged, LastTag 16:47:37 → 16:47:40. Your rig demonstration was right and the reword would have been the wrong call.
The prune really prunes. With the threshold forced to 0 so the gate could fire on a young candidate, docker rmi ran against a real daemon and the sibling tag went; $CI_IMAGE and <none> were never inspected. With the real 172800, a mixed young list is fully kept.
set -e still does not fire on the loop. rc=0 and a container created for: all-young, $CI_IMAGE-only, empty list, docker images failing, inspect failing, rmi failing. *[!0-9]* is also doing double duty as the guard that keeps a non-numeric answer out of $(( )), which is worth the comment it does not have.
Both hatches, for real. env -i with a PATH genuinely lacking sha256sum: bare exits 1 with the "no second digest" text; with DENO_GIT=optional it warns and runs denoland/deno:2.5.6 with no image inspect and no build. Composition holds.
The tag does not move. Recomputed both formulas against docker/Dockerfile at this commit: f4224312806e885c from sha256sum | cut -c1-16 and f4224312806e885c from main's sha256sum || cksum → tr → cut. No fleet-wide rebuild rides along.
docker build -q still surfaces why. I had assumed -q would swallow the mirror failure and leave the operator only your message. It does not — BuildKit prints the offending RUN and exit code: 100 on stderr. The new hard failure is diagnosable.
The new test actually runs. Worth stating because a file nothing executes is decoration: deno task ci → deno task test takes no path filter, so it walks tools/ and picks up ci_pins_test.ts. Confirmed by running the task's exact flag set — 1 passed | 452 filtered out.
deps.yml's opt-out is safe for the firmer reason. All three commands are --allow-read/--allow-net/--allow-env only; none passes --allow-run. Good that this is the sentence in the comment now.
Gates re-run here: deno fmt --check (314), deno lint (196), sh -n, tools/ci_pins_test.ts (3 passed).

1. `LastTagTime` is never refreshed on the path an in-use recipe takes

LastTagTime means "last built or tagged". deno.sh's hot path does neither: docker image inspect succeeds and the script goes straight to docker create. Measured on the real daemon — cached run, three seconds apart:

LastTagTime before=1787323902  after=1787323902

So a recipe used on every push for a month still reads a month old, and the first build of any new recipe evicts it. That is the same failure mode you correctly rejected Created for; the gate only protects a recipe built in the last 48 hours, and in the scenario the gate exists for — a sibling branch on a pin that landed weeks ago — the recipe has been used continuously and built once, long ago. The gate does not fire for it.

This is not a no-op change, and I want to be fair about what it bought: LastTagTime does move on the post-eviction rebuild, so the ping-pong converges instead of repeating on every alternation. One forced rebuild per 48-hour window per pin pair, not one per run. That is strictly better than Created. But it is not what docs/dependencies.md now says:

a sibling branch on a different pin holds a recipe that is in use, not dead

and the forced rebuild is still the apt-get one whose failure item 1 of this PR made fatal on the workflow that deploys.

Closing it is one line, because a cache hit is the signal — this recipe is in use — and docker tag X X is what records it. Verified on the daemon: 1787323660 → 1787323663.

if [ "$CI_IMAGE" != "$IMAGE" ]; then
  if docker image inspect "$CI_IMAGE" >/dev/null 2>&1; then
    # A cache hit is this recipe being USED, and the gate below reads
    # LastTagTime — nothing else on this path moves it, so without this a
    # recipe in daily use reads as old as its first build.
    docker tag "$CI_IMAGE" "$CI_IMAGE" >/dev/null 2>&1 || true
  else
    ...build, prune, hatches, unchanged...
  fi
fi

With that, "neither built nor tagged for two days" and "in use, not dead" become the same statement, and the docs sentence is true as written.

One incidental thing a future reader of the harness will hit: LastTagTime is per image, not per tag. Two tags resolving to one image ID share one timestamp, so tagging either refreshes the gate for both. I ran into it directly — a forced-threshold prune kept a candidate I expected to go, because it and the just-built tag were both 0a95e0f2d863. It over-keeps, so it is harmless, and in production distinct pins mean distinct IDs. It belongs in a comment rather than in someone's afternoon.

2. The `deps.yml` half of the new test is satisfied by a comment

Your eight mutations reproduce: env removed from a deps.yml step (red), and step-, job-, workflow-level and inline DENO_GIT in build.yml (all red). The file-text assertion for build.yml is the right call and closes all three spellings I found last round.

The deps.yml half does not hold. Four mutations that remove the opt-out for real and leave the test green:

The opt-out named only in a comment. - name: Test with # DENO_GIT=optional is not needed here and no env: — passes.
deps.yml's own comment block, relocated. Move the existing "DENO_GIT=optional on every step below" paragraph from above - name: Test to inside it and drop the env: — passes. It reads today only because that block lands in the Checkout chunk, and Checkout has no run:.
A trailing comment after run:. # note: DENO_GIT=optional was dropped here — passes.
An unnamed step. - run: sh .forgejo/deno.sh run --allow-read tools/other.ts, no name:, no env — invisible. steps() filters on - name: , so the whole step never reaches the check, and it merges into the previous chunk.

The first three are the cost of the [:=] widening I asked for last round. That is on me for proposing the regex without saying what it gave up — but the trade is real and unremarked: your round-1 design note was explicitly "it matches DENO_GIT: as a YAML line so the comment in deps.yml … is not a false positive", and that property is gone. The invariant is what the step sets, so strip comments before asking.

Both are one hunk. I applied and mutation-tested it here; deno fmt --check, deno lint and the suite pass, and all 14 mutations land in the demanded direction — the four above flip red, the baseline and the quoted DENO_GIT: "optional" stay green:

  // Comments are stripped first: the check is on what the step SETS, and
  // deps.yml's own comment block spells the opt-out out in prose. Split on
  // any list item, not on `- name:` — a step may have no name.
  const steps = (file: string) =>
    Deno.readTextFileSync(file)
      .replace(/^[ \t]*#.*$/gm, "")
      .split(/^(?=\s*- (?:name|run|uses):)/m)
      .filter((step) => /^\s*- (?:name|run|uses):/.test(step));
  const runsDenoSh = (step: string) => {
    const run = step.search(/^\s*-?\s*run:/m);
    return run >= 0 && step.slice(run).includes("deno.sh");
  };

with the label falling back to the run line for a nameless step:

    .map((step) => step.match(/- (?:name: )?(.*)/)![1].trim());

Stripping comments also retires the false red you kept as "safe direction" — a comment naming deno.sh between a step's run: and the next step no longer reaches the check at all, so the chunking stops being load-bearing.

Smaller

A third workflow escapes all three tests. BUILD/DEPS are hardcoded, so .forgejo/workflows/nightly.yml calling deno.sh with DENO_GIT: optional is green — unpinned actions and its own Deno image too, since WORKFLOWS is the same list. That predates this PR, but the invariant this PR adds is "no caller opts out silently", and a new workflow file is the likeliest place it rots. A Deno.readDirSync(".forgejo/workflows") in place of the literal list covers all three tests at once.
Job-level env: in deps.yml is a false red. Consolidating the three step envs into one job-level entry is runtime-correct and turns the test red. Safe direction, and I would not chase it — but it is the natural cleanup someone will try, so it deserves a word in the test's comment.
build.yml cannot document its own abstention. includes("DENO_GIT") bans the token, so # deliberately no DENO_GIT here: this caller needs git fails the test. That is the assertion working as designed, and it is also the one comment a reader most wants at that step. Worth noting in the failure message that the ban is on the literal string, so whoever hits it does not spend ten minutes looking for an env: block.
172800 is bare. "Two days" lives only in prose two comment paragraphs up. PRUNE_AFTER=172800 # 48h reads at the comparison.
Narrow race the prune introduces. Between another run's successful docker image inspect and its docker create, this run can untag that image; id=$(docker create …) is unguarded, so set -e reds that run. It needs two different recipes concurrently on one runner, which is a bump PR racing main, so it is rare and it was rare before the age gate too. Mentioning it so it is a known shape rather than a mystery red.
Fixing 1 as above also removes the disagreement between the docs' "in use, not dead" and what the code measures, so no reword is needed anywhere.

What I still cannot answer: the containerd image store's zero-time LastTagTime (my daemon uses the classic store — the guard is right by construction, and inspect failing and a bogus field both exit 1 and keep, which I did test), and a genuinely 48-hour-old image, since I forced the threshold to 0 instead of waiting.

Close 2, close or reword 1 — and with the one-line docker tag the reword is not needed — and this is done.

Third pass at `d372259`. I did not reuse your harness or mine: this round is a real daemon (docker 29.7.2), the actual `.forgejo/deno.sh` from this commit driving real `docker build` / `docker images` / `docker image inspect` / `docker rmi`, and 14 mutations against `tools/ci_pins_test.ts` rather than your 8. The two items you took are the right fixes and the mechanics reproduce. Item 1 is closer but still does not deliver the sentence the docs claim, and the `deps.yml` half of the new test can be satisfied by prose. ## What reproduces, including the three things nobody had run for real - **The cold path works end-to-end.** This is the one you flagged as unverified. Real daemon, real recipe, real `docker cp` and `docker create`: the derived image builds, the command runs, and `git --version` inside it answers `git version 2.47.3`. No longer a CI-first. - **`{{.Metadata.LastTagTime.Unix}}` renders.** Your open question, answered: `1787323657` on 29.7.2. And a bad template field exits 1, so the `|| continue` genuinely keeps rather than silently reading an empty string. - **`Created` vs `LastTagTime` is exactly as you argued.** Build, `rmi`, sleep 3, rebuild from layer cache: same image ID, `Created=…16:47:35.985850282` unchanged, `LastTag` `16:47:37` → `16:47:40`. Your rig demonstration was right and the reword would have been the wrong call. - **The prune really prunes.** With the threshold forced to 0 so the gate could fire on a young candidate, `docker rmi` ran against a real daemon and the sibling tag went; `$CI_IMAGE` and `<none>` were never inspected. With the real 172800, a mixed young list is fully kept. - **`set -e` still does not fire on the loop.** rc=0 and a container created for: all-young, `$CI_IMAGE`-only, empty list, `docker images` failing, `inspect` failing, `rmi` failing. `*[!0-9]*` is also doing double duty as the guard that keeps a non-numeric answer out of `$(( ))`, which is worth the comment it does not have. - **Both hatches, for real.** `env -i` with a `PATH` genuinely lacking `sha256sum`: bare exits 1 with the "no second digest" text; with `DENO_GIT=optional` it warns and runs `denoland/deno:2.5.6` with no `image inspect` and no `build`. Composition holds. - **The tag does not move.** Recomputed both formulas against `docker/Dockerfile` at this commit: `f4224312806e885c` from `sha256sum | cut -c1-16` and `f4224312806e885c` from main's `sha256sum || cksum` → `tr` → `cut`. No fleet-wide rebuild rides along. - **`docker build -q` still surfaces why.** I had assumed `-q` would swallow the mirror failure and leave the operator only your message. It does not — BuildKit prints the offending `RUN` and `exit code: 100` on stderr. The new hard failure is diagnosable. - **The new test actually runs.** Worth stating because a file nothing executes is decoration: `deno task ci` → `deno task test` takes no path filter, so it walks `tools/` and picks up `ci_pins_test.ts`. Confirmed by running the task's exact flag set — `1 passed | 452 filtered out`. - **`deps.yml`'s opt-out is safe for the firmer reason.** All three commands are `--allow-read`/`--allow-net`/`--allow-env` only; none passes `--allow-run`. Good that this is the sentence in the comment now. - Gates re-run here: `deno fmt --check` (314), `deno lint` (196), `sh -n`, `tools/ci_pins_test.ts` (3 passed). ## 1. `LastTagTime` is never refreshed on the path an in-use recipe takes `LastTagTime` means "last built or tagged". `deno.sh`'s hot path does neither: `docker image inspect` succeeds and the script goes straight to `docker create`. Measured on the real daemon — cached run, three seconds apart: ``` LastTagTime before=1787323902 after=1787323902 ``` So a recipe used on every push for a month still reads a month old, and the first build of any new recipe evicts it. That is the same failure mode you correctly rejected `Created` for; the gate only protects a recipe *built* in the last 48 hours, and in the scenario the gate exists for — a sibling branch on a pin that landed weeks ago — the recipe has been used continuously and built once, long ago. The gate does not fire for it. This is not a no-op change, and I want to be fair about what it bought: `LastTagTime` does move on the post-eviction rebuild, so the ping-pong converges instead of repeating on every alternation. One forced rebuild per 48-hour window per pin pair, not one per run. That is strictly better than `Created`. But it is not what `docs/dependencies.md` now says: > a sibling branch on a different pin holds a recipe that is in use, not dead and the forced rebuild is still the `apt-get` one whose failure item 1 of this PR made fatal on the workflow that deploys. Closing it is one line, because a cache hit *is* the signal — this recipe is in use — and `docker tag X X` is what records it. Verified on the daemon: `1787323660` → `1787323663`. ```sh if [ "$CI_IMAGE" != "$IMAGE" ]; then if docker image inspect "$CI_IMAGE" >/dev/null 2>&1; then # A cache hit is this recipe being USED, and the gate below reads # LastTagTime — nothing else on this path moves it, so without this a # recipe in daily use reads as old as its first build. docker tag "$CI_IMAGE" "$CI_IMAGE" >/dev/null 2>&1 || true else ...build, prune, hatches, unchanged... fi fi ``` With that, "neither built nor tagged for two days" and "in use, not dead" become the same statement, and the docs sentence is true as written. One incidental thing a future reader of the harness will hit: `LastTagTime` is per **image**, not per tag. Two tags resolving to one image ID share one timestamp, so tagging either refreshes the gate for both. I ran into it directly — a forced-threshold prune kept a candidate I expected to go, because it and the just-built tag were both `0a95e0f2d863`. It over-keeps, so it is harmless, and in production distinct pins mean distinct IDs. It belongs in a comment rather than in someone's afternoon. ## 2. The `deps.yml` half of the new test is satisfied by a comment Your eight mutations reproduce: env removed from a `deps.yml` step (red), and step-, job-, workflow-level and inline `DENO_GIT` in `build.yml` (all red). The file-text assertion for `build.yml` is the right call and closes all three spellings I found last round. The `deps.yml` half does not hold. Four mutations that remove the opt-out for real and leave the test **green**: - **The opt-out named only in a comment.** `- name: Test` with `# DENO_GIT=optional is not needed here` and no `env:` — passes. - **`deps.yml`'s own comment block, relocated.** Move the existing "``DENO_GIT=optional`` on every step below" paragraph from above `- name: Test` to inside it and drop the `env:` — passes. It reads today only because that block lands in the Checkout chunk, and Checkout has no `run:`. - **A trailing comment after `run:`.** `# note: DENO_GIT=optional was dropped here` — passes. - **An unnamed step.** `- run: sh .forgejo/deno.sh run --allow-read tools/other.ts`, no `name:`, no env — invisible. `steps()` filters on `- name: `, so the whole step never reaches the check, and it merges into the previous chunk. The first three are the cost of the `[:=]` widening I asked for last round. That is on me for proposing the regex without saying what it gave up — but the trade is real and unremarked: your round-1 design note was explicitly "it matches `DENO_GIT:` as a YAML line so the comment in deps.yml … is not a false positive", and that property is gone. The invariant is what the step *sets*, so strip comments before asking. Both are one hunk. I applied and mutation-tested it here; `deno fmt --check`, `deno lint` and the suite pass, and all 14 mutations land in the demanded direction — the four above flip red, the baseline and the quoted `DENO_GIT: "optional"` stay green: ```ts // Comments are stripped first: the check is on what the step SETS, and // deps.yml's own comment block spells the opt-out out in prose. Split on // any list item, not on `- name:` — a step may have no name. const steps = (file: string) => Deno.readTextFileSync(file) .replace(/^[ \t]*#.*$/gm, "") .split(/^(?=\s*- (?:name|run|uses):)/m) .filter((step) => /^\s*- (?:name|run|uses):/.test(step)); const runsDenoSh = (step: string) => { const run = step.search(/^\s*-?\s*run:/m); return run >= 0 && step.slice(run).includes("deno.sh"); }; ``` with the label falling back to the run line for a nameless step: ```ts .map((step) => step.match(/- (?:name: )?(.*)/)![1].trim()); ``` Stripping comments also retires the false red you kept as "safe direction" — a comment naming `deno.sh` between a step's `run:` and the next step no longer reaches the check at all, so the chunking stops being load-bearing. ## Smaller - **A third workflow escapes all three tests.** `BUILD`/`DEPS` are hardcoded, so `.forgejo/workflows/nightly.yml` calling `deno.sh` with `DENO_GIT: optional` is green — unpinned actions and its own Deno image too, since `WORKFLOWS` is the same list. That predates this PR, but the invariant this PR adds is "no caller opts out silently", and a new workflow file is the likeliest place it rots. A `Deno.readDirSync(".forgejo/workflows")` in place of the literal list covers all three tests at once. - **Job-level `env:` in `deps.yml` is a false red.** Consolidating the three step envs into one job-level entry is runtime-correct and turns the test red. Safe direction, and I would not chase it — but it is the natural cleanup someone will try, so it deserves a word in the test's comment. - **`build.yml` cannot document its own abstention.** `includes("DENO_GIT")` bans the token, so `# deliberately no DENO_GIT here: this caller needs git` fails the test. That is the assertion working as designed, and it is also the one comment a reader most wants at that step. Worth noting in the failure message that the ban is on the literal string, so whoever hits it does not spend ten minutes looking for an `env:` block. - **`172800` is bare.** "Two days" lives only in prose two comment paragraphs up. `PRUNE_AFTER=172800 # 48h` reads at the comparison. - **Narrow race the prune introduces.** Between another run's successful `docker image inspect` and its `docker create`, this run can untag that image; `id=$(docker create …)` is unguarded, so `set -e` reds that run. It needs two different recipes concurrently on one runner, which is a bump PR racing main, so it is rare and it was rare before the age gate too. Mentioning it so it is a known shape rather than a mystery red. - Fixing 1 as above also removes the disagreement between the docs' "in use, not dead" and what the code measures, so no reword is needed anywhere. What I still cannot answer: the containerd image store's zero-time `LastTagTime` (my daemon uses the classic store — the guard is right by construction, and `inspect` failing and a bogus field both exit 1 and keep, which I did test), and a genuinely 48-hour-old image, since I forced the threshold to 0 instead of waiting. Close 2, close or reword 1 — and with the one-line `docker tag` the reword is not needed — and this is done.

julian added 1 commit

2026-08-21 19:46:13 +02:00

ci(deno.sh): using an image keeps it, and the opt-out must be set, not mentioned

Build and Deploy / verify (pull_request) Successful in 1m21s

Details

Build and Deploy / build (pull_request) Has been skipped

Details

Dependency Check / dependencies (pull_request) Successful in 1m16s

Details

e18994dadd

The prune gate reads LastTagTime, and nothing on the cached path moved
it — inspect and create touch no timestamp — so a recipe in daily use
read as old as its first build and the first build of any new recipe
evicted it. A self-tag (docker tag X X) on the cache hit is a no-op to
the image and moves exactly that field, so "neither built nor tagged
for two days" and "in use, not dead" are now the same statement, and
the docs sentence is true as written.

tools/ci_pins_test.ts now strips comments before asking — deps.yml's
own comment block spells the opt-out in prose, and a comment must
satisfy nothing and trip nothing — and splits steps on any opening key,
so an unnamed `- run:` step is seen instead of merging into its
neighbour. The workflow list is discovered from .forgejo/workflows/
rather than hardcoded: a new workflow cannot ship an unpinned action,
its own Deno image, or a silent DENO_GIT opt-out; the opt-out is earned
by joining the per-step check deps.yml is under.

Also: PRUNE_AFTER=172800 named at the comparison; the non-numeric
guard's second job, the per-image LastTagTime footgun and the narrow
inspect-vs-prune race written down where they will be hit; build.yml's
failure message says the ban is on the literal string so nobody hunts
for an env: block; the job-level-env false red documented as the
deliberate trade it is.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

julian commented

2026-08-21 19:47:17 +02:00

Round 4 at e18994d — both items taken as you specified, plus the smaller ones.

1. The self-tag, exactly as proposed. The cached path now branches on a successful docker image inspect and runs docker tag "$CI_IMAGE" "$CI_IMAGE" || true with your comment's reasoning attached. "Neither built nor tagged for two days" and "in use, not dead" are now the same statement, so the docs sentence stands unreworded. The per-image-not-per-tag footgun is written down next to the gate. Honesty note: no daemon on this machine, so the LastTagTime movement itself rests on your measurements (1787323660 → 1787323663); what I verified against the stub is placement — a cache hit tags exactly once, builds nothing, prunes nothing; a failing tag does not red the run; the cold path never self-tags.

2. Your hunk, applied whole. Comments stripped before asking, steps split on - (name|run|uses):, filter matching the split, label falling back to the run line, and the build.yml half generalized: the file-text ban now applies to every workflow except deps.yml, with WORKFLOWS discovered via Deno.readDirSync(".forgejo/workflows") — your "third workflow" smaller item, folded into the same loop so nightly.yml with DENO_GIT: optional is red until it joins the per-step check deliberately. That is a step beyond what you asked (you asked for readDir on the list; the ban-outside-deps.yml rule is my generalization of "no caller opts out silently") — flagging it in case you want the rule narrower.

Your four green-mutation escapes all flip red now: comment-only opt-out, the relocated deps.yml comment block, a trailing comment after run:, and the unnamed step. Ten mutations total this round, all in the demanded direction — including quoted "optional" and a stray deno.sh comment staying green (the chunking is no longer load-bearing), and job-level env consolidation as the false red the test comment now documents as deliberate.

Smaller, all taken: PRUNE_AFTER=172800 # 48h at the comparison; the *[!0-9]* guard's second job ($(( )) protection) commented; the inspect-vs-prune race named in the prune preamble as a known shape; build.yml's failure message says the ban is on the literal string, comments included, so nobody hunts for an env: block.

Gates: deno fmt --check (314), deno lint (196), deno task check, tools/ 58 passed (ci_pins_test.ts 3 + dep-check 55), sh -n. Server suite: 109 passed, 1 failed — the known interval leak in external_modules_test.ts, which this diff does not touch. Recipe tag still tf4224312806e885c. PR description updated to match.

What I could not check here, same as before: anything needing a real daemon — the containerd zero-time branch and a genuine 48-hour-old image remain open from your list too.

Round 4 at e18994d — both items taken as you specified, plus the smaller ones. **1. The self-tag, exactly as proposed.** The cached path now branches on a successful `docker image inspect` and runs `docker tag "$CI_IMAGE" "$CI_IMAGE" || true` with your comment's reasoning attached. "Neither built nor tagged for two days" and "in use, not dead" are now the same statement, so the docs sentence stands unreworded. The per-image-not-per-tag footgun is written down next to the gate. Honesty note: no daemon on this machine, so the `LastTagTime` movement itself rests on your measurements (`1787323660 → 1787323663`); what I verified against the stub is placement — a cache hit tags exactly once, builds nothing, prunes nothing; a failing `tag` does not red the run; the cold path never self-tags. **2. Your hunk, applied whole.** Comments stripped before asking, steps split on `- (name|run|uses):`, filter matching the split, label falling back to the run line, and the `build.yml` half generalized: the file-text ban now applies to **every** workflow except `deps.yml`, with `WORKFLOWS` discovered via `Deno.readDirSync(".forgejo/workflows")` — your "third workflow" smaller item, folded into the same loop so `nightly.yml` with `DENO_GIT: optional` is red until it joins the per-step check deliberately. That is a step beyond what you asked (you asked for readDir on the list; the ban-outside-deps.yml rule is my generalization of "no caller opts out silently") — flagging it in case you want the rule narrower. Your four green-mutation escapes all flip red now: comment-only opt-out, the relocated `deps.yml` comment block, a trailing comment after `run:`, and the unnamed step. Ten mutations total this round, all in the demanded direction — including quoted `"optional"` and a stray `deno.sh` comment staying green (the chunking is no longer load-bearing), and job-level env consolidation as the false red the test comment now documents as deliberate. **Smaller, all taken:** `PRUNE_AFTER=172800 # 48h` at the comparison; the `*[!0-9]*` guard's second job (`$(( ))` protection) commented; the inspect-vs-prune race named in the prune preamble as a known shape; `build.yml`'s failure message says the ban is on the literal string, comments included, so nobody hunts for an `env:` block. Gates: `deno fmt --check` (314), `deno lint` (196), `deno task check`, `tools/` 58 passed (`ci_pins_test.ts` 3 + dep-check 55), `sh -n`. Server suite: 109 passed, 1 failed — the known interval leak in `external_modules_test.ts`, which this diff does not touch. Recipe tag still `tf4224312806e885c`. PR description updated to match. What I could not check here, same as before: anything needing a real daemon — the containerd zero-time branch and a genuine 48-hour-old image remain open from your list too.

julian requested review from thisilike

2026-08-21 19:47:19 +02:00

thisilike requested changes

2026-08-21 20:33:52 +02:00

Dismissed

thisilike left a comment

Fourth pass at e18994d. Real daemon (docker 29.7.2), the actual script from this commit driving real build / images / image inspect / rmi / tag, plus a stub docker for the failure paths and 18 mutations against tools/ci_pins_test.ts rather than your 10. Both items you took are the right fixes and every mechanic in the description reproduces, including the docker tag X X measurement. Two things do not hold: the prune only runs on the one path where its own gate guarantees there is nothing to collect, and the deps.yml half of the test still has three silent opt-outs — one of them a key deps.yml steps already carry.

What reproduces

The cold path, end to end. Real recipe, real docker cp / create: opsdeck-deno-ci:tf4224312806e885c builds and the 55 tools/dep-check tests run inside it, rc=0, 21 s wall.
The tag does not move. Recomputed both formulas against docker/Dockerfile at this commit: tf4224312806e885c from sha256sum | cut -c1-16 and tf4224312806e885c from main's sha256sum || cksum → tr → cut. cksum alone gives t1622011007241, so the mixed-fleet claim is real, not theoretical.
The self-tag moves LastTagTime, and it is the only thing on that path that does. Direct: 1787336766 → 1787336769 across docker tag X X, Created untouched. Through the script: a cache-hit run moved 1787336818 → 1787336840 in 0.8 s with no build. The stub call log for that path is exactly image inspect / tag / create / cp / start / wait / rm.
{{.Metadata.LastTagTime.Unix}} renders and a bad field exits 1, so || continue genuinely keeps rather than reading an empty string.
The prune prunes, and is scoped. Threshold forced to 0 against the real daemon: both sibling tags rmi'd, the tag just built kept, denoland/deno never a candidate, rc=0. Two sibling tags sharing one image ID both went — the per-image caveat over-keeps only when a candidate shares an ID with something young.
set -e still does not fire. rc=0 and a container created for: all-young list, $CI_IMAGE-only, empty list, docker images failing, inspect failing, rmi failing, tag failing, non-numeric LastTagTime. The only rc=1 in the matrix is the intended one.
Both hatches, for real. env -i with a PATH genuinely lacking sha256sum: bare exits 1 with the "no second digest" text; with DENO_GIT=optional it warns and runs the base image with no image inspect and no build.
Discovery closes the third-workflow escape. A new nightly.yml goes red for the opt-out, for an unpinned uses:, and for its own container: image: denoland/deno — three separate mutations. One calling deno.sh without the opt-out stays green, which is the correct default.
The build.yml half holds in every spelling I could find. Step env:, job-level env:, workflow-level env:, inline on the run line, and a comment merely naming DENO_GIT — all red.
The test runs under the task. deno task test's exact flag set over tools/: 58 passed, ci_pins_test.ts included. Gates re-run here: deno fmt --check (314), deno lint (196), sh -n, tools/ci_pins_test.ts (3 passed).
One thing I checked because set -e is load-bearing in this file and it is fine: the for name in ${DENO_ENV:-} loop ends in [ -n "$value" ] && env_args=…, so the loop's status is 1 whenever the last name is unset. Neither sh nor busybox ash exits on it — the AND-OR exemption carries through the loop — so the Check dependencies and file issues step is safe. Pre-existing and untouched; no action.

1. The prune runs only on the path where the gate guarantees nothing to collect

The loop is inside the build elif (.forgejo/deno.sh:110), so it runs only when a recipe is cold-built. Measured: threshold forced to 0, one sibling tag aged three seconds, cache-hit run — the sibling survives. Only a cold build sweeps.

Now put that next to the 48-hour gate. At the moment a cold build happens, the recipe it just superseded is by definition young: something used it within the last 48 hours, which is why the bump is landing today. So:

bump A→B lands, main cold-builds B, prune pass sees A — used an hour ago — and keeps it.
nothing cold-builds again until the next recipe change, so no pass comes past A at all.
A is collected at the next bump. Weeks later, months if the pin is quiet.

The one pass that could collect A always arrives too early, and the passes that would find it old never happen. In the ping-pong case the gate correctly keeps both live recipes — that half works — but the issue's actual complaint ("nothing removed superseded derived images") is still answered one generation late, and the runner steady-states at current + previous, which on a toolchain bump is a whole extra Deno image rather than the extra apt layer.

Closing it is a hoist, and it makes the age gate the whole policy: a recipe stops being self-tagged 48 h after its last use, and then the next run of anything at all collects it. It is also strictly safer on the race named at :113 — a candidate is by definition something nothing has tagged for two days, and any concurrent user would have self-tagged it on the way in.

prune_stale() {
  PRUNE_AFTER=172800 # 48h
  now=$(date +%s)
  ...loop unchanged...
}

if [ "$CI_IMAGE" != "$IMAGE" ]; then
  if docker image inspect "$CI_IMAGE" >/dev/null 2>&1; then
    docker tag "$CI_IMAGE" "$CI_IMAGE" >/dev/null 2>&1 || true
    prune_stale
  elif printf '%s\n' "$RECIPE" | docker build -q -t "$CI_IMAGE" - >/dev/null; then
    prune_stale
  elif [ "$GIT_POLICY" = optional ]; then

Applied and measured here on the real daemon with the same forced threshold: the aged sibling is pruned on the cache-hit path, rc=0, sh -n clean, the run itself unaffected. docs/dependencies.md then reads "on every run" rather than "after a successful build".

2. Three step shapes still opt out silently — `if:` among them

steps() splits on - (?:name|run|uses):, so a step whose first key is anything else never opens a chunk: it merges into the previous step's, and if that one is opted out, the new one inherits the pass. Mutations against deps.yml, all three green:

- if: github.event_name == 'pull_request' + run: sh .forgejo/deno.sh run …, no env. if: is a key two deps.yml steps already carry — just never first.
- env: first, holding some other variable, no DENO_GIT.
- id: first.

All three genuinely require git at runtime. It is also still position-dependent: the same step placed after Checkout (no run: in that chunk) goes red — so the chunking is load-bearing again, which the round-3 note said stripping comments had retired.

Both the code comment at :68 ("Split on any list item that can open a step") and the description ("steps split on any opening key so an unnamed - run: step is seen") describe the fix that isn't there: name|run|uses is three keys, not any. Two lines:

      .split(/^(?=\s*- [\w-]+:)/m)
      .filter((step) => /^\s*- [\w-]+:/.test(step));

on: schedule: - cron: starts a chunk under that pattern and is harmlessly dropped by runsDenoSh. Verified with the patch in place: the three above flip red, the baseline stays green, and the other 15 mutations land exactly as they do today — quoted DENO_GIT: "optional" green, inline DENO_GIT=optional sh … green, unnamed - run: red, comment-only red, job-level env: red (the documented false red), all four build.yml spellings red, all three nightly.yml shapes red. deno fmt --check and deno lint clean.

3. `DENO_GIT: optionally` passes the test and requires git at runtime

/DENO_GIT[:=]\s*["']?optional/ has no right-hand boundary, so any value that merely starts with optional satisfies it. GIT_POLICY=optionally is not optional, so that step needs git — the silent skip this test exists to prevent, spelled with a typo, green. optional(?![\w-]) closes it and keeps both the quoted and the inline forms green; verified in the same matrix.

Smaller

The prune's own failure mode is silence. An inspect that cannot answer .Metadata.LastTagTime — the containerd image store's zero time, as the comment says, or any other reason — keeps every candidate, rc=0, nothing on stderr. Verified. Item 1 of this PR exists because "green with the work silently not done" is the bug; one echo "warning: no LastTagTime from this daemon; not pruning" >&2 on the first skip would keep item 2 from being the same shape. Not a blocker.
docs/dependencies.md:115 — "every deno.sh step in deps.yml carries the env" is the sentence finding 2 falsifies. Nothing to reword once the split is fixed.
PRUNE_AFTER=172800 # 48h at the comparison: taken, and it reads.
The description's "no other workflow names DENO_GIT anywhere in its text, in any spelling" is true of the repo's files, and that is the whole guarantee: the wrapper reads the process environment, so a runner exporting DENO_GIT=optional globally opts every caller out with all three tests green. Nothing a text assertion can reach — worth knowing where the edge is rather than fixing.
The rejected first option still reads as the weakest paragraph, and I still am not asking for it. commit_test.ts's ignore: !hasGit is untouched, so what this buys is "CI refuses to run without git" and not "the suite cannot silently skip" — which is the right guarantee, and findings 1–3 are the places where the wrapper-and-test arrangement does not yet deliver it.

Fix 1 and 2 — both are single hunks I have applied and measured here — and 3 rides along on the same regex. Then this is done.

Fourth pass at `e18994d`. Real daemon (docker 29.7.2), the actual script from this commit driving real `build` / `images` / `image inspect` / `rmi` / `tag`, plus a stub `docker` for the failure paths and 18 mutations against `tools/ci_pins_test.ts` rather than your 10. Both items you took are the right fixes and every mechanic in the description reproduces, including the `docker tag X X` measurement. Two things do not hold: the prune only runs on the one path where its own gate guarantees there is nothing to collect, and the `deps.yml` half of the test still has three silent opt-outs — one of them a key `deps.yml` steps already carry. ## What reproduces - **The cold path, end to end.** Real recipe, real `docker cp` / `create`: `opsdeck-deno-ci:tf4224312806e885c` builds and the 55 `tools/dep-check` tests run inside it, rc=0, 21 s wall. - **The tag does not move.** Recomputed both formulas against `docker/Dockerfile` at this commit: `tf4224312806e885c` from `sha256sum | cut -c1-16` and `tf4224312806e885c` from main's `sha256sum || cksum` → `tr` → `cut`. `cksum` alone gives `t1622011007241`, so the mixed-fleet claim is real, not theoretical. - **The self-tag moves `LastTagTime`, and it is the only thing on that path that does.** Direct: `1787336766` → `1787336769` across `docker tag X X`, `Created` untouched. Through the script: a cache-hit run moved `1787336818` → `1787336840` in 0.8 s with no build. The stub call log for that path is exactly `image inspect` / `tag` / `create` / `cp` / `start` / `wait` / `rm`. - **`{{.Metadata.LastTagTime.Unix}}` renders and a bad field exits 1**, so `|| continue` genuinely keeps rather than reading an empty string. - **The prune prunes, and is scoped.** Threshold forced to 0 against the real daemon: both sibling tags `rmi`'d, the tag just built kept, `denoland/deno` never a candidate, rc=0. Two sibling tags sharing one image ID both went — the per-image caveat over-keeps only when a candidate shares an ID with something young. - **`set -e` still does not fire.** rc=0 and a container created for: all-young list, `$CI_IMAGE`-only, empty list, `docker images` failing, `inspect` failing, `rmi` failing, `tag` failing, non-numeric `LastTagTime`. The only rc=1 in the matrix is the intended one. - **Both hatches, for real.** `env -i` with a `PATH` genuinely lacking `sha256sum`: bare exits 1 with the "no second digest" text; with `DENO_GIT=optional` it warns and runs the base image with no `image inspect` and no `build`. - **Discovery closes the third-workflow escape.** A new `nightly.yml` goes red for the opt-out, for an unpinned `uses:`, and for its own `container: image: denoland/deno` — three separate mutations. One calling `deno.sh` *without* the opt-out stays green, which is the correct default. - **The `build.yml` half holds in every spelling I could find.** Step `env:`, job-level `env:`, workflow-level `env:`, inline on the run line, and a comment merely naming `DENO_GIT` — all red. - **The test runs under the task.** `deno task test`'s exact flag set over `tools/`: 58 passed, `ci_pins_test.ts` included. Gates re-run here: `deno fmt --check` (314), `deno lint` (196), `sh -n`, `tools/ci_pins_test.ts` (3 passed). - One thing I checked because `set -e` is load-bearing in this file and it is fine: the `for name in ${DENO_ENV:-}` loop ends in `[ -n "$value" ] && env_args=…`, so the loop's status is 1 whenever the last name is unset. Neither `sh` nor busybox `ash` exits on it — the AND-OR exemption carries through the loop — so the `Check dependencies and file issues` step is safe. Pre-existing and untouched; no action. ## 1. The prune runs only on the path where the gate guarantees nothing to collect The loop is inside the build `elif` (`.forgejo/deno.sh:110`), so it runs only when a recipe is *cold-built*. Measured: threshold forced to 0, one sibling tag aged three seconds, cache-hit run — the sibling survives. Only a cold build sweeps. Now put that next to the 48-hour gate. At the moment a cold build happens, the recipe it just superseded is *by definition* young: something used it within the last 48 hours, which is why the bump is landing today. So: - bump A→B lands, main cold-builds B, prune pass sees A — used an hour ago — and keeps it. - nothing cold-builds again until the *next* recipe change, so no pass comes past A at all. - A is collected at the next bump. Weeks later, months if the pin is quiet. The one pass that could collect A always arrives too early, and the passes that would find it old never happen. In the ping-pong case the gate correctly keeps both live recipes — that half works — but the issue's actual complaint ("nothing removed superseded derived images") is still answered one generation late, and the runner steady-states at current + previous, which on a toolchain bump is a whole extra Deno image rather than the extra apt layer. Closing it is a hoist, and it makes the age gate the whole policy: a recipe stops being self-tagged 48 h after its last use, and then the next run of anything at all collects it. It is also strictly *safer* on the race named at `:113` — a candidate is by definition something nothing has tagged for two days, and any concurrent user would have self-tagged it on the way in. ```sh prune_stale() { PRUNE_AFTER=172800 # 48h now=$(date +%s) ...loop unchanged... } if [ "$CI_IMAGE" != "$IMAGE" ]; then if docker image inspect "$CI_IMAGE" >/dev/null 2>&1; then docker tag "$CI_IMAGE" "$CI_IMAGE" >/dev/null 2>&1 || true prune_stale elif printf '%s\n' "$RECIPE" | docker build -q -t "$CI_IMAGE" - >/dev/null; then prune_stale elif [ "$GIT_POLICY" = optional ]; then ``` Applied and measured here on the real daemon with the same forced threshold: the aged sibling is pruned on the cache-hit path, rc=0, `sh -n` clean, the run itself unaffected. `docs/dependencies.md` then reads "on every run" rather than "after a successful build". ## 2. Three step shapes still opt out silently — `if:` among them `steps()` splits on `- (?:name|run|uses):`, so a step whose *first* key is anything else never opens a chunk: it merges into the previous step's, and if that one is opted out, the new one inherits the pass. Mutations against `deps.yml`, all three **green**: - `- if: github.event_name == 'pull_request'` + `run: sh .forgejo/deno.sh run …`, no env. `if:` is a key two `deps.yml` steps already carry — just never first. - `- env:` first, holding some other variable, no `DENO_GIT`. - `- id:` first. All three genuinely require git at runtime. It is also still position-dependent: the same step placed after Checkout (no `run:` in that chunk) goes red — so the chunking is load-bearing again, which the round-3 note said stripping comments had retired. Both the code comment at `:68` ("Split on any list item that can open a step") and the description ("steps split on any opening key so an unnamed `- run:` step is seen") describe the fix that isn't there: `name|run|uses` is three keys, not any. Two lines: ```ts .split(/^(?=\s*- [\w-]+:)/m) .filter((step) => /^\s*- [\w-]+:/.test(step)); ``` `on: schedule: - cron:` starts a chunk under that pattern and is harmlessly dropped by `runsDenoSh`. Verified with the patch in place: the three above flip red, the baseline stays green, and the other 15 mutations land exactly as they do today — quoted `DENO_GIT: "optional"` green, inline `DENO_GIT=optional sh …` green, unnamed `- run:` red, comment-only red, job-level `env:` red (the documented false red), all four `build.yml` spellings red, all three `nightly.yml` shapes red. `deno fmt --check` and `deno lint` clean. ## 3. `DENO_GIT: optionally` passes the test and requires git at runtime `/DENO_GIT[:=]\s*["']?optional/` has no right-hand boundary, so any value that merely starts with `optional` satisfies it. `GIT_POLICY=optionally` is not `optional`, so that step needs git — the silent skip this test exists to prevent, spelled with a typo, green. `optional(?![\w-])` closes it and keeps both the quoted and the inline forms green; verified in the same matrix. ## Smaller - **The prune's own failure mode is silence.** An `inspect` that cannot answer `.Metadata.LastTagTime` — the containerd image store's zero time, as the comment says, or any other reason — keeps every candidate, rc=0, nothing on stderr. Verified. Item 1 of this PR exists because "green with the work silently not done" is the bug; one `echo "warning: no LastTagTime from this daemon; not pruning" >&2` on the first skip would keep item 2 from being the same shape. Not a blocker. - `docs/dependencies.md:115` — "every `deno.sh` step in `deps.yml` carries the env" is the sentence finding 2 falsifies. Nothing to reword once the split is fixed. - `PRUNE_AFTER=172800 # 48h` at the comparison: taken, and it reads. - The description's "no other workflow names `DENO_GIT` anywhere in its text, in any spelling" is true of the repo's files, and that is the whole guarantee: the wrapper reads the process environment, so a runner exporting `DENO_GIT=optional` globally opts every caller out with all three tests green. Nothing a text assertion can reach — worth knowing where the edge is rather than fixing. - The rejected first option still reads as the weakest paragraph, and I still am not asking for it. `commit_test.ts`'s `ignore: !hasGit` is untouched, so what this buys is "CI refuses to run without git" and not "the suite cannot silently skip" — which is the right guarantee, and findings 1–3 are the places where the wrapper-and-test arrangement does not yet deliver it. Fix 1 and 2 — both are single hunks I have applied and measured here — and 3 rides along on the same regex. Then this is done.

julian added 1 commit

2026-08-21 22:07:15 +02:00

ci(deno.sh): a sweep that only follows a build always arrives too early

Build and Deploy / verify (pull_request) Successful in 2m13s

Details

Build and Deploy / build (pull_request) Has been skipped

Details

Dependency Check / dependencies (pull_request) Successful in 41s

Details

751c701d38

The prune ran only after a cold build — the one moment the tag it just
superseded is young by the gate, since something needed it until that day.
So the one pass that could collect a stale tag kept it, none came past
again until the next recipe change, and the runner steady-stated at
current + previous. prune_stale() now also runs from the cache-hit path:
a tag stops being self-tagged 48h after its last use, and then the next
run of anything collects it. That also narrows the untag-vs-create race —
a candidate is by definition something nothing self-tagged for two days.
A daemon that cannot answer LastTagTime now warns once instead of being
a prune that silently never prunes.

tools/ci_pins_test.ts: steps() splits on ANY key that can open a list
item, not on name|run|uses — a step led by `if:`, `env:` or `id:` used to
merge into its predecessor's chunk and inherit its opt-out. And the
opt-out value must end at `optional`: `DENO_GIT: optionally` is not
`optional` to the wrapper, so it now reads as missing rather than set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

julian commented

2026-08-21 22:08:07 +02:00

Round-5 rework at 751c701, all of round 4 taken — both blockers, the regex rider, and the warn-once Also-item.

1. The prune runs from both paths now. Your hoist, almost verbatim: the loop is prune_stale(), called right after the self-tag on the cache-hit path and after a successful cold build. The comment block moved up with it and now carries the one-generation-early reasoning — at the moment of a cold build, the tag it supersedes is young by the gate, so a build-only sweep always kept it and none came past again until the next bump. It also notes what the hoist buys the race at the old :113: a candidate is by definition something nothing self-tagged for two days, so any concurrent user would have refreshed it on the way in. docs/dependencies.md now says "every run that holds a derived image" instead of "after a successful build".

2. steps() splits on any opening key. Your two lines: .split(/^(?=\s*- [\w-]+:)/m) + the matching filter. Your three green mutations flip red — - if: first (including placed directly after Checkout, so the chunking is no longer load-bearing), - env: first holding another variable, - id: first. The on: schedule: - cron: item opens a chunk and is dropped by runsDenoSh, as you predicted; noted in the comment.

3. optional(?![\w-]). DENO_GIT: optionally and inline DENO_GIT=optionally both red; quoted and inline optional both still green.

Also: the un-ageable-candidate case warns once — warning: no LastTagTime from this daemon; not pruning on the first skip, then silence. The inspect failure and the non-numeric answer land in the same warned-and-kept branch (|| last= replaced || continue), so a daemon that never answers is no longer a prune that silently never prunes.

Verification (stub CLI here; your rounds 2–4 own the real-daemon semantics): 28-check stub matrix — cache hit self-tags once + prunes a stale sibling with no build, young sibling kept, cold build prunes and skips <none> and the tag just built, two un-ageable candidates produce exactly one warning line (the warned flag lives in the loop's single subshell, so it persists across iterations), negative zero-time kept, images/rmi/tag failures never red a run, both hatches unchanged in both directions. 16 mutations against the test, all in the demanded direction — your four new reds plus my round-4 set, with the quoted form, the inline opt-out, a nightly.yml calling the wrapper without the opt-out, and the baseline staying green, and job-level consolidation staying the documented false red. Gates: deno task check, deno lint (196), deno fmt --check (314), tools/ 58 passed, sh -n. The recipe text is untouched, so the tag is tf4224312806e885c by construction.

Description updated to match. Over to you.

Round-5 rework at 751c701, all of round 4 taken — both blockers, the regex rider, and the warn-once Also-item. **1. The prune runs from both paths now.** Your hoist, almost verbatim: the loop is `prune_stale()`, called right after the self-tag on the cache-hit path and after a successful cold build. The comment block moved up with it and now carries the one-generation-early reasoning — at the moment of a cold build, the tag it supersedes is young by the gate, so a build-only sweep always kept it and none came past again until the next bump. It also notes what the hoist buys the race at the old `:113`: a candidate is by definition something nothing self-tagged for two days, so any concurrent user would have refreshed it on the way in. `docs/dependencies.md` now says "every run that holds a derived image" instead of "after a successful build". **2. `steps()` splits on any opening key.** Your two lines: `.split(/^(?=\s*- [\w-]+:)/m)` + the matching filter. Your three green mutations flip red — `- if:` first (including placed directly after Checkout, so the chunking is no longer load-bearing), `- env:` first holding another variable, `- id:` first. The `on: schedule: - cron:` item opens a chunk and is dropped by `runsDenoSh`, as you predicted; noted in the comment. **3. `optional(?![\w-])`.** `DENO_GIT: optionally` and inline `DENO_GIT=optionally` both red; quoted and inline `optional` both still green. **Also:** the un-ageable-candidate case warns once — `warning: no LastTagTime from this daemon; not pruning` on the first skip, then silence. The inspect failure and the non-numeric answer land in the same warned-and-kept branch (`|| last=` replaced `|| continue`), so a daemon that never answers is no longer a prune that silently never prunes. Verification (stub CLI here; your rounds 2–4 own the real-daemon semantics): 28-check stub matrix — cache hit self-tags once + prunes a stale sibling with no build, young sibling kept, cold build prunes and skips `<none>` and the tag just built, two un-ageable candidates produce exactly one warning line (the `warned` flag lives in the loop's single subshell, so it persists across iterations), negative zero-time kept, `images`/`rmi`/`tag` failures never red a run, both hatches unchanged in both directions. 16 mutations against the test, all in the demanded direction — your four new reds plus my round-4 set, with the quoted form, the inline opt-out, a `nightly.yml` calling the wrapper *without* the opt-out, and the baseline staying green, and job-level consolidation staying the documented false red. Gates: `deno task check`, `deno lint` (196), `deno fmt --check` (314), `tools/` 58 passed, `sh -n`. The recipe text is untouched, so the tag is `tf4224312806e885c` by construction. Description updated to match. Over to you.

julian requested review from thisilike

2026-08-21 22:08:09 +02:00

thisilike approved these changes

2026-08-21 23:08:07 +02:00

thisilike left a comment

Fifth pass at 751c701. Same method as round 4 and not your harness: real daemon (docker 29.7.2) driving the actual script from this commit, my own stub for the failure paths, and 29 mutations against tools/ci_pins_test.ts rather than your 16. All three findings are closed, the warn-once you added on top does what it says, and I could not find a new hole in either half. Approving.

The three, re-measured

1 — the sweep now runs on the path that matters. prune_stale() at .forgejo/deno.sh:133, called from both branches. Real daemon, threshold forced to 0, sibling tag aged three seconds, cache-hit run: the sibling is rmi'd, rc=0, the run itself unaffected — the exact case that survived at e18994d. With the real 172800 a sibling tagged seconds ago is kept and the run costs 1.4 s instead of 0.8 s, which is the whole price of the change. Cold path re-run end to end afterwards: untagged, rebuilt from layer cache in 2.6 s, pruned the other sibling, kept its own tag, 55 tools/dep-check tests pass inside the derived image.
2 — the step splitter. All six opening keys I could construct now land red: - if: (both after the opted-out step and first in the job, so position no longer matters), - env: holding another variable, - id:, - shell:, - continue-on-error:, - working-directory:, plus the unnamed - run:. The - cron: chunk your comment calls out does start a chunk and is dropped by runsDenoSh, as claimed. A nested - key: list item inside a step ahead of its run: splits the env away from the run line and reads as missing — a false red, the safe direction, and contrived enough not to be worth a word.
3 — the value boundary. DENO_GIT: optionally, DENO_GIT: optional-ish and inline DENO_GIT=optionally are red; DENO_GIT: "optional", 'optional', optional # trailing comment, env: { DENO_GIT: optional } and the inline DENO_GIT=optional run line stay green. Runtime and test agree on every one of those.

Full matrix this round, 29 mutations, all in the demanded direction: the thirteen above, comment-only opt-out and env-removed red, job-level env: still the documented false red, build.yml step / job / workflow / inline / comment-mention all red, nightly.yml red for the opt-out, for an unpinned uses: and for its own container: image:, and green when it calls the wrapper without the opt-out. Baseline green.

The warn-once

Verified rather than assumed, because a flag set inside a while fed by a pipe is exactly the shape that silently doesn't persist: two candidates the daemon cannot age produce one warning line, rc=0, nothing pruned. Mixed list — one unreadable, one 48 h+, one young — warns once, rmis only the old one, keeps the young one. The containerd zero time (-6795364578871) and an empty answer both land in the same branch and warn. docker images failing, rmi failing, tag failing, an empty list, an all-young list: rc=0 every time, container still created. The only rc=1 in the matrix is the intended build failure without the opt-out, and DENO_GIT=optional still turns it into a warning on the base image. Cold path takes no self-tag, hot path takes exactly one.

Gates here: sh -n, deno fmt --check (314), deno lint (196), tools/ci_pins_test.ts (3 passed), and the derived-image run of tools/dep-check (55 passed). Tag is still tf4224312806e885c, so nothing rebuilds fleet-wide on merge.

Two nits, neither blocking

The warning names the wrong cause when the tag simply vanished. last=$(docker image inspect …) || last= funnels every non-zero inspect into the "no LastTagTime from this daemon" branch, and No such image exits 1 the same way a missing template field does (checked). The concurrent-prune race the comment names at :104 produces precisely that: the other run untags between this run's docker images and its inspect, and the log then accuses the daemon. Wording, not behaviour — "could not read LastTagTime for $stale; not pruning it" covers both causes and stays true.
That race is now reachable from every run rather than only after a cold build. Named in the comment already, and the exposure is still only the microseconds between one run's inspect and its rmi, against a tag nothing has self-tagged for two days — the loser is a retriable red, and the alternative was a sweep that never collected anything. Worth knowing it went from rare-and-after-builds to rare-and-always; not worth changing.

The cost of the hoist is real and correctly bounded: a recipe whose branch runs less often than every 48 hours now loses its image to somebody else's run and pays a rebuild — 2.6 s while the builder cache holds the layers, the real apt-get once a runner reclaims it. That is the age gate working as designed, and docs/dependencies.md says so in the sentence about what "in use" means.

Good change. Merge it.

Fifth pass at `751c701`. Same method as round 4 and not your harness: real daemon (docker 29.7.2) driving the actual script from this commit, my own stub for the failure paths, and 29 mutations against `tools/ci_pins_test.ts` rather than your 16. All three findings are closed, the warn-once you added on top does what it says, and I could not find a new hole in either half. Approving. ## The three, re-measured - **1 — the sweep now runs on the path that matters.** `prune_stale()` at `.forgejo/deno.sh:133`, called from both branches. Real daemon, threshold forced to 0, sibling tag aged three seconds, **cache-hit** run: the sibling is `rmi`'d, rc=0, the run itself unaffected — the exact case that survived at `e18994d`. With the real 172800 a sibling tagged seconds ago is kept and the run costs 1.4 s instead of 0.8 s, which is the whole price of the change. Cold path re-run end to end afterwards: untagged, rebuilt from layer cache in 2.6 s, pruned the other sibling, kept its own tag, 55 `tools/dep-check` tests pass inside the derived image. - **2 — the step splitter.** All six opening keys I could construct now land red: `- if:` (both after the opted-out step *and* first in the job, so position no longer matters), `- env:` holding another variable, `- id:`, `- shell:`, `- continue-on-error:`, `- working-directory:`, plus the unnamed `- run:`. The `- cron:` chunk your comment calls out does start a chunk and is dropped by `runsDenoSh`, as claimed. A nested `- key:` list item inside a step ahead of its `run:` splits the env away from the run line and reads as missing — a false red, the safe direction, and contrived enough not to be worth a word. - **3 — the value boundary.** `DENO_GIT: optionally`, `DENO_GIT: optional-ish` and inline `DENO_GIT=optionally` are red; `DENO_GIT: "optional"`, `'optional'`, `optional # trailing comment`, `env: { DENO_GIT: optional }` and the inline `DENO_GIT=optional` run line stay green. Runtime and test agree on every one of those. Full matrix this round, 29 mutations, all in the demanded direction: the thirteen above, comment-only opt-out and env-removed red, job-level `env:` still the documented false red, `build.yml` step / job / workflow / inline / comment-mention all red, `nightly.yml` red for the opt-out, for an unpinned `uses:` and for its own `container: image:`, and green when it calls the wrapper *without* the opt-out. Baseline green. ## The warn-once Verified rather than assumed, because a flag set inside a `while` fed by a pipe is exactly the shape that silently doesn't persist: two candidates the daemon cannot age produce **one** warning line, rc=0, nothing pruned. Mixed list — one unreadable, one 48 h+, one young — warns once, `rmi`s only the old one, keeps the young one. The containerd zero time (`-6795364578871`) and an empty answer both land in the same branch and warn. `docker images` failing, `rmi` failing, `tag` failing, an empty list, an all-young list: rc=0 every time, container still created. The only rc=1 in the matrix is the intended build failure without the opt-out, and `DENO_GIT=optional` still turns it into a warning on the base image. Cold path takes no self-tag, hot path takes exactly one. Gates here: `sh -n`, `deno fmt --check` (314), `deno lint` (196), `tools/ci_pins_test.ts` (3 passed), and the derived-image run of `tools/dep-check` (55 passed). Tag is still `tf4224312806e885c`, so nothing rebuilds fleet-wide on merge. ## Two nits, neither blocking - **The warning names the wrong cause when the tag simply vanished.** `last=$(docker image inspect …) || last=` funnels every non-zero inspect into the "no LastTagTime from this daemon" branch, and `No such image` exits 1 the same way a missing template field does (checked). The concurrent-prune race the comment names at `:104` produces precisely that: the other run untags between this run's `docker images` and its `inspect`, and the log then accuses the daemon. Wording, not behaviour — "could not read LastTagTime for `$stale`; not pruning it" covers both causes and stays true. - **That race is now reachable from every run rather than only after a cold build.** Named in the comment already, and the exposure is still only the microseconds between one run's `inspect` and its `rmi`, against a tag nothing has self-tagged for two days — the loser is a retriable red, and the alternative was a sweep that never collected anything. Worth knowing it went from rare-and-after-builds to rare-and-always; not worth changing. The cost of the hoist is real and correctly bounded: a recipe whose branch runs less often than every 48 hours now loses its image to somebody else's run and pays a rebuild — 2.6 s while the builder cache holds the layers, the real `apt-get` once a runner reclaims it. That is the age gate working as designed, and `docs/dependencies.md` says so in the sentence about what "in use" means. Good change. Merge it.

julian merged commit 3b8e539c11 into main

2026-08-21 23:20:26 +02:00

julian referenced this pull request from a commit

2026-08-21 23:20:26 +02:00

Merge pull request 'ci(deno.sh): losing git is an error, one digest, and no image left behind' (#42) from fix/issue-29-deno-sh-followups into main

julian referenced this pull request

2026-08-21 23:21:42 +02:00

deno.sh: prune warning blames the daemon when the tag simply vanished in the concurrent-prune race #56

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

OpsDeck/core!42

Rows
Columns

ci(deno.sh): losing git is an error, one digest, and no image left behind #42

1. The soft fallback re-opened the silent-skip hole

2. Nothing removed superseded derived images

3. RECIPE_KEY was not stable across runners

Verification

What holds up

1. The prune and the hard failure compound into cache thrash

2. DENO_GIT=optional still dies on a missing sha256sum

3. The verification is not in the tree

Smaller

What reproduces

1. The gate protects the recipe first built recently, not the one in use

2. build.yml can still opt out — three ways, one of them the documented spelling

Smaller

1. The gate is LastTagTime now

2. build.yml cannot opt out in any spelling

Smaller

Verification

What reproduces, including the three things nobody had run for real

1. LastTagTime is never refreshed on the path an in-use recipe takes

2. The deps.yml half of the new test is satisfied by a comment

Smaller

What reproduces

1. The prune runs only on the path where the gate guarantees nothing to collect

2. Three step shapes still opt out silently — if: among them

3. DENO_GIT: optionally passes the test and requires git at runtime

Smaller

The three, re-measured

The warn-once

Two nits, neither blocking

3. `RECIPE_KEY` was not stable across runners

2. `DENO_GIT=optional` still dies on a missing `sha256sum`

2. `build.yml` can still opt out — three ways, one of them the documented spelling

1. The gate is `LastTagTime` now

2. `build.yml` cannot opt out in any spelling

1. `LastTagTime` is never refreshed on the path an in-use recipe takes

2. The `deps.yml` half of the new test is satisfied by a comment

2. Three step shapes still opt out silently — `if:` among them

3. `DENO_GIT: optionally` passes the test and requires git at runtime