ci: pin the whole deploy path, and watch it weekly #11

Merged
julian merged 14 commits from ci/dependency-update-issues into main 2026-08-10 15:46:07 +02:00
Owner

Comes out of the pinning thread on #10, and replaces the part of it that was going to be a policy document nobody reads.

Three commits, one mechanism: pin everything the deploy path trusts, and run something that stops a hard pin from going stale. Either half alone is worse than useless — a checker over movable tags watches nothing, and a pin with nothing watching it rots into a two-year-old checkout action.

The problem, restated

Deno's dependencies are watched: deno.json, a lockfile, deno outdated, and deno install --frozen in the image build. Nothing underneath that was. .forgejo/workflows/build.yml builds the image and then SSHes to the production host, so everything it runs holds PACKAGE_TOKEN and DEPLOY_SSH_KEY while it runs — and every input to that job was named by something movable:

  • actions by tag — actions/checkout@v6 was whatever that repository decided it meant on the morning it ran;
  • base images by tag — same, repushable;
  • and the deploy target by ssh-keyscan on every run, which trusted whatever answered on that address at that moment, one line before handing it the credential.

Pinning fixes all three and creates one new problem: pins go stale silently, which is the actual reason nobody pins.

e56579a — the watcher

tools/dep-check reads .forgejo/workflows/*.yml and docker/Dockerfile, asks each registry or forge which tags exist, and files an issue per dependency that is behind. .forgejo/workflows/deps.yml runs it Mondays at 06:00 UTC, plus workflow_dispatch.

Built to survive being ignored, because a weekly job that files a fresh issue every run is a job people mute, and a muted job is worse than no job — it looks like coverage:

  • one issue per dependency, identified by a marker in the body rather than the title (the title carries the version, so matching on it would open a second issue the first time a project ships a release);
  • edited, not duplicated, when the newest version moves;
  • a closed issue is a decision. Not reopened. Nothing more is said about that dependency until something newer than the version that was declined ships;
  • when the repository catches up, the run comments and closes the issue itself;
  • a run that finds nothing new writes nothing at all.

Matching is shape first, order second. The non-numeric parts of a tag are a deployment choice and not a version, so bin-2.5.6 is never offered as an upgrade to 2.5.6, a prerelease is only offered to something already on one, and bookworm-slim is reported as having no version to compare rather than as up to date — the distinction is the difference between a checker and a checker that lies.

Within one shape a missing component counts as zero, so v6 is behind v6.0.1. That is the opposite of the rule the docker module wants for image pins, and the reason is written next to it: these are releases, v6 and v6.0.0 are the same one.

06cd02f — the version pins

Where What
.forgejo/workflows/build.yml actions/checkout, docker/login-action, docker/setup-buildx-action, docker/build-push-action → full commit SHAs
.forgejo/workflows/deps.yml actions/checkout → SHA, and the denoland/deno container it runs in → digest
docker/Dockerfile denoland/deno (build), debian:bookworm-slim (runtime), denoland/deno:bin-… (the COPY --from) → index digests

Pinned at the versions already declared, not the newest. Moving checkout from v6 to v7 is a decision; the point of the weekly check is that it arrives as an issue rather than as a tag that moved on its own. The first run says so about six of the seven, which makes this PR self-demonstrating.

Both hosts agree. code.forgejo.org and github.com returned identical SHAs for all four action tags, so the pin does not depend on which one the instance resolves from.

The trailing # v6 comments are load-bearing. A SHA says nothing about which release it is, so discover.ts reads the comment — that is what keeps the checker answering after its own advice is taken. A pin whose comment lies is worse than no comment, which is why re-pinning moves both in one commit.

One thing the pinning turned up: COPY --from=denoland/deno:bin-2.5.6 is a base image that appears in no FROM line, so nothing listed it and discover.ts did not see it. It would have stayed unpinned and unwatched. Both fixed here, with a test.

856d4b7 — the host key

ssh-keyscan -H 192.168.0.3 >> known_hosts is gone. Trust-on-first-use where every use is a first use is not trust; it is a formality that produces a known_hosts file. The key is pinned in .forgejo/known_hosts, and the deploy states StrictHostKeyChecking=yes rather than relying on a default a runner image could have changed. A substituted host now fails verification instead of collecting the credential.

In the repository rather than in a secret: a host public key is public by design, so committing it makes it reviewable and makes a change to it show up in a diff. Only the ed25519 key is listed — it is what the client prefers, and the RSA and ECDSA keys the host also offers would each be another way for verification to pass after this one was replaced.

Confirmed twice before being written down: it matches the entry already in the operator's known_hosts, and it matches what the host reports for itself over an already-trusted session. Then verified with nothing else on the path:

$ ssh -o StrictHostKeyChecking=yes -o UserKnownHostsFile=.forgejo/known_hosts \
      -o BatchMode=yes jul14n@192.168.0.3 'echo host-key-verification-ok; hostname'
host-key-verification-ok
epyclab

(The key's comment field still reads root@rx200s6 — a stale label from an earlier machine, not a mismatch. The key itself is what the live host presents.)

A rebuild or a rekey now fails the deploy closed, which is the right outcome. Both the file and docs/security.md say how to re-pin.

Verification

Local; there is no CI on this branch, because on: pull_request is added by #10 and deps.yml deliberately has no PR trigger.

deno fmt / deno lint     9 files clean
deno check               tools/dep-check/main.ts
deno test                26 passed, 0 failed
docker buildx build --check -f docker/Dockerfile .
                         all three digests resolved against the registry,
                         "Check complete, no warnings found"
ssh with only .forgejo/known_hosts on the path — verified, connected

And the loop closes — the checker still finds everything through the pins:

$ deno run --allow-read --allow-net --allow-env tools/dep-check/main.ts --dry-run
checking 7 dependencies
  + actions/checkout v6: v7.0.1 available, filing an issue
  = debian bookworm-slim: no version in that tag, nothing to compare
  + denoland/deno bin-2.5.6: bin-2.9.5 available, filing an issue
  + denoland/deno 2.5.6: 2.9.5 available, filing an issue
  + docker/build-push-action v6: v7.3.0 available, filing an issue
  + docker/login-action v3: v4.6.0 available, filing an issue
  + docker/setup-buildx-action v3: v4.2.0 available, filing an issue
dry run: nothing was written

Six real findings on the first run. That is the backlog this repository already had and could not see. --dry-run does every read and no write, and needs no token.

Scope

Deliberately narrow. tools/dep-check does not watch deno.json: deno outdated already answers for that, and a second opinion on the same question is a second thing to keep correct. Adding a kind is a small change if that turns out to be wanted.

Notes for review

  • deno.json gains "@std/assert": "jsr:@std/assert@^1" in the import map, because deno lint refuses an inline jsr: specifier and the new tests need assertions. #10 adds the identical line in the identical place, so the two should merge without a conflict — the one line here worth watching when they meet.
  • Forge.label() looks the dependencies label up and files without it when absent. It does not create labels: a token that may only write issues should not fail a whole run over one, and a label nobody made is a label nobody filters on.
  • Where uses: owner/repo@ref resolves is an instance setting, so hosts are tried in order — code.forgejo.org then github.com, overridable with DEP_CHECK_ACTION_HOSTS. Whichever answered is recorded in the issue body.
  • docs/dependencies.md carries what is pinned and the re-pinning procedure with exact commands; docs/security.md gains a section on the build and deploy path. Owner: whoever merges the dependency issue.
  • First deploy after this merges is the one to watch. If the host key were wrong the deploy fails closed rather than doing something dangerous, but it would fail.
Comes out of the pinning thread on #10, and replaces the part of it that was going to be a policy document nobody reads. Three commits, one mechanism: **pin everything the deploy path trusts, and run something that stops a hard pin from going stale.** Either half alone is worse than useless — a checker over movable tags watches nothing, and a pin with nothing watching it rots into a two-year-old checkout action. ### The problem, restated Deno's dependencies are watched: `deno.json`, a lockfile, `deno outdated`, and `deno install --frozen` in the image build. Nothing underneath that was. `.forgejo/workflows/build.yml` builds the image and then SSHes to the production host, so everything it runs holds `PACKAGE_TOKEN` and `DEPLOY_SSH_KEY` while it runs — and every input to that job was named by something movable: - actions by tag — `actions/checkout@v6` was whatever that repository decided it meant on the morning it ran; - base images by tag — same, repushable; - and the deploy target by **`ssh-keyscan` on every run**, which trusted whatever answered on that address at that moment, one line before handing it the credential. Pinning fixes all three and creates one new problem: pins go stale silently, which is the actual reason nobody pins. ### `e56579a` — the watcher `tools/dep-check` reads `.forgejo/workflows/*.yml` and `docker/Dockerfile`, asks each registry or forge which tags exist, and files an issue per dependency that is behind. `.forgejo/workflows/deps.yml` runs it Mondays at 06:00 UTC, plus `workflow_dispatch`. Built to survive being ignored, because a weekly job that files a fresh issue every run is a job people mute, and a muted job is worse than no job — it looks like coverage: - **one issue per dependency**, identified by a marker in the body rather than the title (the title carries the version, so matching on it would open a second issue the first time a project ships a release); - **edited**, not duplicated, when the newest version moves; - **a closed issue is a decision.** Not reopened. Nothing more is said about that dependency until something newer than the version that was declined ships; - when the repository catches up, the run comments and closes the issue itself; - **a run that finds nothing new writes nothing at all.** Matching is shape first, order second. The non-numeric parts of a tag are a deployment choice and not a version, so `bin-2.5.6` is never offered as an upgrade to `2.5.6`, a prerelease is only offered to something already on one, and `bookworm-slim` is reported as *having no version to compare* rather than as up to date — the distinction is the difference between a checker and a checker that lies. Within one shape a missing component counts as zero, so `v6` is behind `v6.0.1`. That is the **opposite** of the rule the docker module wants for image pins, and the reason is written next to it: these are releases, `v6` and `v6.0.0` are the same one. ### `06cd02f` — the version pins | Where | What | | --- | --- | | `.forgejo/workflows/build.yml` | `actions/checkout`, `docker/login-action`, `docker/setup-buildx-action`, `docker/build-push-action` → full commit SHAs | | `.forgejo/workflows/deps.yml` | `actions/checkout` → SHA, and the `denoland/deno` container it runs in → digest | | `docker/Dockerfile` | `denoland/deno` (build), `debian:bookworm-slim` (runtime), `denoland/deno:bin-…` (the `COPY --from`) → index digests | **Pinned at the versions already declared, not the newest.** Moving `checkout` from v6 to v7 is a decision; the point of the weekly check is that it arrives as an issue rather than as a tag that moved on its own. The first run says so about six of the seven, which makes this PR self-demonstrating. **Both hosts agree.** `code.forgejo.org` and `github.com` returned identical SHAs for all four action tags, so the pin does not depend on which one the instance resolves from. **The trailing `# v6` comments are load-bearing.** A SHA says nothing about which release it is, so `discover.ts` reads the comment — that is what keeps the checker answering after its own advice is taken. A pin whose comment lies is worse than no comment, which is why re-pinning moves both in one commit. **One thing the pinning turned up:** `COPY --from=denoland/deno:bin-2.5.6` is a base image that appears in no `FROM` line, so nothing listed it and `discover.ts` did not see it. It would have stayed unpinned *and* unwatched. Both fixed here, with a test. ### `856d4b7` — the host key `ssh-keyscan -H 192.168.0.3 >> known_hosts` is gone. Trust-on-first-use where every use is a first use is not trust; it is a formality that produces a `known_hosts` file. The key is pinned in `.forgejo/known_hosts`, and the deploy states `StrictHostKeyChecking=yes` rather than relying on a default a runner image could have changed. A substituted host now fails verification instead of collecting the credential. In the repository rather than in a secret: a host public key is public by design, so committing it makes it reviewable and makes a change to it show up in a diff. Only the ed25519 key is listed — it is what the client prefers, and the RSA and ECDSA keys the host also offers would each be another way for verification to pass after this one was replaced. Confirmed twice before being written down: it matches the entry already in the operator's `known_hosts`, and it matches what the host reports for itself over an already-trusted session. Then verified with nothing else on the path: ``` $ ssh -o StrictHostKeyChecking=yes -o UserKnownHostsFile=.forgejo/known_hosts \ -o BatchMode=yes jul14n@192.168.0.3 'echo host-key-verification-ok; hostname' host-key-verification-ok epyclab ``` (The key's comment field still reads `root@rx200s6` — a stale label from an earlier machine, not a mismatch. The key itself is what the live host presents.) A rebuild or a rekey now fails the deploy closed, which is the right outcome. Both the file and `docs/security.md` say how to re-pin. ### Verification Local; there is no CI on this branch, because `on: pull_request` is added by #10 and `deps.yml` deliberately has no PR trigger. ``` deno fmt / deno lint 9 files clean deno check tools/dep-check/main.ts deno test 26 passed, 0 failed docker buildx build --check -f docker/Dockerfile . all three digests resolved against the registry, "Check complete, no warnings found" ssh with only .forgejo/known_hosts on the path — verified, connected ``` And the loop closes — the checker still finds everything *through* the pins: ``` $ deno run --allow-read --allow-net --allow-env tools/dep-check/main.ts --dry-run checking 7 dependencies + actions/checkout v6: v7.0.1 available, filing an issue = debian bookworm-slim: no version in that tag, nothing to compare + denoland/deno bin-2.5.6: bin-2.9.5 available, filing an issue + denoland/deno 2.5.6: 2.9.5 available, filing an issue + docker/build-push-action v6: v7.3.0 available, filing an issue + docker/login-action v3: v4.6.0 available, filing an issue + docker/setup-buildx-action v3: v4.2.0 available, filing an issue dry run: nothing was written ``` Six real findings on the first run. That is the backlog this repository already had and could not see. `--dry-run` does every read and no write, and needs no token. ### Scope Deliberately narrow. `tools/dep-check` does **not** watch `deno.json`: `deno outdated` already answers for that, and a second opinion on the same question is a second thing to keep correct. Adding a kind is a small change if that turns out to be wanted. ### Notes for review - `deno.json` gains `"@std/assert": "jsr:@std/assert@^1"` in the import map, because `deno lint` refuses an inline `jsr:` specifier and the new tests need assertions. **#10 adds the identical line in the identical place**, so the two should merge without a conflict — the one line here worth watching when they meet. - `Forge.label()` looks the `dependencies` label up and files without it when absent. It does not create labels: a token that may only write issues should not fail a whole run over one, and a label nobody made is a label nobody filters on. - Where `uses: owner/repo@ref` resolves is an instance setting, so hosts are tried in order — `code.forgejo.org` then `github.com`, overridable with `DEP_CHECK_ACTION_HOSTS`. Whichever answered is recorded in the issue body. - `docs/dependencies.md` carries what is pinned and the re-pinning procedure with exact commands; `docs/security.md` gains a section on the build and deploy path. **Owner:** whoever merges the dependency issue. - First deploy after this merges is the one to watch. If the host key were wrong the deploy fails closed rather than doing something dangerous, but it would fail.
Deno's dependencies have `deno outdated`, a lockfile and a `--frozen`
install. The layer underneath CI has none of that: the actions a workflow
runs and the images it runs them in are named by tags, a tag is a movable
pointer rather than a version, and nothing was watching them. That cuts both
ways — a tag can move under us, and a tag we pin can go stale for a year
without anybody noticing.

So: `tools/dep-check` reads `.forgejo/workflows/*.yml` and `docker/Dockerfile`,
asks each registry or forge which tags exist, and puts the answer where
decisions actually get made. `.forgejo/workflows/deps.yml` runs it every
Monday morning.

Built to survive being ignored, because a weekly job that files a fresh issue
every run is a job people mute, and a muted job is worse than no job — it
looks like coverage:

- one issue per dependency, edited when the newest version moves, never
  duplicated;
- an issue closed without upgrading is a decision. It is not reopened, and
  nothing more is said about that dependency until something newer than the
  version that was declined ships;
- when the repository catches up, the run comments and closes the issue;
- a run that finds nothing new writes nothing at all.

Matching is shape-first: the non-numeric parts of a tag are a deployment
choice, not a version, so `bin-2.5.6` is never offered as an upgrade to
`2.5.6` and `bookworm-slim` is reported as having no version to compare
rather than as up to date. Within a shape a missing component counts as zero,
so `v6` is behind `v6.0.1` — the opposite of the docker module's rule, and
the reason is written down next to it: these are releases, not image pins.

A commit-pinned action keeps answering through its `# v6` trailing comment,
so the checker still works after its own advice is taken. Digest-pinned
images keep their tag, so they need nothing.

Verified against the real registries: 6 dependencies discovered, five behind
(checkout v7.0.1, deno 2.9.5, three docker actions), one unversioned.
`--dry-run` does every read and no write.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A checker over movable tags watches nothing: the previous commit could tell
you `actions/checkout` had moved on, while `@v6` still meant whatever that
repository decided it meant on the morning it ran. The pin and the check are
one mechanism, and shipping half of it was the wrong half.

Actions go to full commit SHAs with the tag kept in a trailing comment; base
images get their index digest alongside the tag. Both hosts a Forgejo
instance might resolve actions from — code.forgejo.org and github.com —
returned identical SHAs for all four tags, so the pin does not depend on
which one answers.

Pinned at the versions that were already declared, not at the newest. Moving
`checkout` from v6 to v7 is a decision, and the point of the weekly check is
that it arrives as an issue rather than as a tag that moved on its own. The
first run will say so about six of the seven.

`COPY --from=denoland/deno:bin-2.5.6` in the Dockerfile turned out to be a
base image that no list of base images contained — it appears in no FROM
line, so discover.ts did not see it and it would have stayed unpinned and
unwatched. Both fixed here.

The trailing comments are load-bearing rather than decoration: a SHA says
nothing about which release it is, so the checker reads the comment to keep
answering after its own advice is taken. A pin whose comment lies is worse
than no comment, which is why re-pinning moves both in one commit.

Verified: `docker buildx build --check` resolves all three digests against
the registry and lints clean; the dry run still discovers all seven
dependencies through their pins and reports six behind.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
julian changed title from ci: ask every week whether anything we pin has moved to ci: pin every action and base image, and watch them weekly 2026-08-10 12:35:03 +02:00
`ssh-keyscan -H 192.168.0.3 >> known_hosts` ran on every deploy, which means
the job trusted whatever answered on that address at that moment — one line
before handing it DEPLOY_SSH_KEY. Trust-on-first-use where every use is a
first use is not trust; it is a formality that produces a known_hosts file.

The host key is pinned in .forgejo/known_hosts now, and the deploy states
StrictHostKeyChecking=yes rather than relying on a default a runner image
could have changed. A substituted host fails verification instead of
collecting the credential.

In the repository rather than in a secret: a host public key is public by
design, so committing it makes it reviewable and makes a change to it show up
in a diff. Only the ed25519 key is listed — it is what the client prefers,
and the RSA and ECDSA keys the host also offers would each be another way for
verification to pass after this one was replaced.

The key was confirmed twice before being written down: it matches the entry
already in the operator's known_hosts, and it matches what the host reports
for itself over an already-trusted session. Verified against the live host
with nothing else on the path:

  ssh -o StrictHostKeyChecking=yes -o UserKnownHostsFile=.forgejo/known_hosts \
      -o BatchMode=yes jul14n@192.168.0.3 hostname

A rebuild or a rekey now fails the deploy closed, which is the right outcome;
both the file and docs/security.md say how to re-pin.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
julian changed title from ci: pin every action and base image, and watch them weekly to ci: pin the whole deploy path, and watch it weekly 2026-08-10 12:42:07 +02:00
thisilike requested changes 2026-08-10 12:58:09 +02:00
Dismissed
thisilike left a comment

Reviewed at 06cd02f against base 1ae4844, in a clean worktree. Every inline comment below sits on a line this pull request adds — nothing here is a complaint about code that was already there. Ran the lot: deno fmt --check / deno lint / deno check tools/dep-check/main.ts clean, 26 tests pass, and the dry run reproduces the output in the description exactly.

The pinning half is correct, and I verified it against the registries rather than taking the description's word for it. All four action SHAs resolve to the tag in their trailing comment on both github.com and code.forgejo.org, byte-identical; all three image digests match the current manifest for their tag:

actions/checkout@v6            [both hosts] d23441a48e516b6c34aea4fa41551a30e30af803 == PIN
docker/login-action@v3         [both hosts] c94ce9fb468520275223c153574b00df6fe4bcc9 == PIN
docker/setup-buildx-action@v3  [both hosts] 8d2750c68a42422c14e847fe6c8ac0403b4cbd6f == PIN
docker/build-push-action@v6    [both hosts] 10e90e3645eae34f1e60eeb005ba3a3d33f178e8 == PIN
denoland/deno:2.5.6            sha256:3ea71953…f166aa == PIN
denoland/deno:bin-2.5.6        sha256:2f5f9d65…d94c7  == PIN
debian:bookworm-slim           sha256:abd67ffc…29241  == PIN

The shape-first version matching in version.ts is genuinely careful — sha256-… rejected as a prefix, bin- never offered against plain, prerelease gated on the current tag. No complaints there.

The checker half has three bugs that fire on this repository, not on a hypothetical one.


Blockers

1. Issue identity is the name alone, and two dependencies here share a name

The marker holds name + latest; main.ts matches on name=<dep.name> . But Dependency identity is kind + name + version, and the dry run finds two denoland/deno entries — 2.5.6 and bin-2.5.6. After the first run both issues carry <!-- dep-check name=denoland/deno … -->, so each dependency matches both issues and decide takes whichever open one comes first.

Reproduced against the real decide, with two issues as the first run would leave them:

dep denoland/deno 2.5.6     -> {"do":"update","issue":102}   # 102 is the bin issue
dep denoland/deno bin-2.5.6 -> {"do":"skip","why":"already reported as bin-2.9.5"}

The bin issue gets rewritten with the plain image's title and body; the plain issue is orphaned and never touched again. Next Monday the marker says 2.9.5, so it flips back. It ping-pongs one issue between two dependencies indefinitely, and the second issue is never closed even after the upgrade lands.

The same collision runs through resolve: upgrading one variant closes the other variant's issue, and that close is then treated as a decision.

Fix is small — put the declared version in the marker (name=… from=… latest=…) and match on name + version.

2. latest === null means two different things, and one of them closes issues

decide reads a null latest as "the repository caught up". It is also what you get from a registry that answers 200 {"tags": []}, from a truncated page, and from a trailing comment the regex read as junk (# renovate: v6 yields renovate:). Reproduced:

pickLatest('v6', []) = null
decide -> {"do":"resolve","issue":4}

That path comments "no newer tag of that shape exists" and closes the issue. Combined with the closed-issue rule — a decision, never reopened — a single bad answer permanently silences that dependency for that version. Nothing writes to the log to say it happened.

resolve needs positive evidence: a non-empty tag list that contains the declared tag. Absence of a result is not the same as absence of a newer version, and this tool's own argument is that the distinction is what separates a checker from a checker that lies.

3. The 1000-tag cap is real and silent

Measured against the live registry:

library/debian: 30 pages, 2976 tags   # tool reads 1000, stops at "jessie-20200130"
denoland/deno:  returns exactly 1000  # cap hit, stops at "debian-1.24.3"

Both cap out. Docker Hub sorts lexically, so the numeric and bin- tags happen to land before the cutoff and today's answers are right by accident. Nothing warns when the cap is hit, so the first tag that sorts past it turns "up to date" into a guess with the same confident output.

Cheapest correct fix: when the loop exits with url still set, return an errormain already skips those and reports them.


Serious

  1. Actions never paginate at all (remote.ts) — one request, link ignored. Counts today: checkout 68, login-action 43, build-push-action 74. Under the cap, but the same silent class as #3, and GitHub's /tags ordering is not a documented sort.

  2. No error handling around any forge write (main.ts) — a single non-2xx from create/update/comment/close escapes main() and kills the run, leaving the remaining dependencies unprocessed. That directly contradicts the rule stated fifteen lines above it: "one unreachable host must not cost the other twelve their issue." The rule is right; it just is not applied to the write side.

  3. The entire mutating half is untested and has never executed. issues_test.ts covers decide; Forge has no test. deps.yml has no pull_request or push trigger and Forgejo only schedules from the default branch, so nothing on this branch has exercised the workflow, the container, the token, or checkout inside a Deno image. The first run of create/update/comment/close will be unattended, against the live tracker — and it hits #1 on its second execution. DEP_CHECK_FORGE_URL / DEP_CHECK_REPOSITORY exist precisely so this can be rehearsed against a scratch repository; worth doing before merge, or adding a Forge test with an injected fetch.

  4. docs/dependencies.md opens by stating something that is not true of this repository — that CI installs with --frozen. Nothing here passes it; the Dockerfile's install step is plain deno install, which resolves past a stale lock rather than failing on it. The description repeats the claim. In a document whose entire job is to be accurate about what is watched and what is not, this is the one line that is not. Adding --frozen to the Dockerfile is probably the better of the two fixes — same argument as the rest of this change, one layer up — but dropping the sentence also works.

  5. deps.yml has no permissions: block. A pull request about narrowing supply-chain blast radius gives the job the runner's default full-repository token in order to file issues. contents: read + issues: write is one line, and it is the argument the rest of the change makes.


Moderate

  1. update fires on a change to latest.tag and never on a change to dep.version. Bump v6v6.5 while v7 is still out and the issue keeps saying "declared at v6 and v7 exists", with the title v6 → v7, indefinitely. Reproduced.

  2. A SHA-pinned uses: with no trailing comment is dropped with no output at all — the exact failure this change exists to prevent, met with silence.

  3. .forgejo/workflows and docker/Dockerfile are hardcoded single paths. The COPY --from hole this pull request just fixed was the same class of bug: a place nobody had listed.

  4. Forge.issues() caps at 10 × 50 = 500 with state=all and stops silently; past that the tool stops finding its own issues and refiles duplicates.

  5. existing is a single snapshot taken before the loop, so a create in one iteration is invisible to the next. Compounds #1.

Minor

  • workflowImagesFrom matches image: at any nesting, including a with: input.
  • The FROM regex anchors on $, so FROM x:1 # comment is silently skipped.
  • SHA is 40 hex only; git's sha256 object format is 64.
  • reports.sort keys on dep.name alone, so the two denoland/deno rows swap order between runs.
  • (outside this diff, flagged because this change is what makes it bite) deno task check still only walks packages/server/main.ts, so a type error in the new tools/dep-check fails neither the image build nor the push to main — only the Monday job. That is what the "must fail here, not on Monday" comment in deps.yml is reaching for.

Verdict

Worth splitting. 06cd02f — the pins — is correct, independently verified, and good to merge on its own. e56579a — the checker — should not be pointed at the live tracker until #1, #2 and #3 are fixed and the write path has been run once against a scratch repository.

Everything above is reproducible; happy to hand over the probe scripts for #1, #2 and #3.

Reviewed at `06cd02f` against base `1ae4844`, in a clean worktree. Every inline comment below sits on a line this pull request adds — nothing here is a complaint about code that was already there. Ran the lot: `deno fmt --check` / `deno lint` / `deno check tools/dep-check/main.ts` clean, 26 tests pass, and the dry run reproduces the output in the description exactly. **The pinning half is correct, and I verified it against the registries rather than taking the description's word for it.** All four action SHAs resolve to the tag in their trailing comment on *both* `github.com` and `code.forgejo.org`, byte-identical; all three image digests match the current manifest for their tag: ``` actions/checkout@v6 [both hosts] d23441a48e516b6c34aea4fa41551a30e30af803 == PIN docker/login-action@v3 [both hosts] c94ce9fb468520275223c153574b00df6fe4bcc9 == PIN docker/setup-buildx-action@v3 [both hosts] 8d2750c68a42422c14e847fe6c8ac0403b4cbd6f == PIN docker/build-push-action@v6 [both hosts] 10e90e3645eae34f1e60eeb005ba3a3d33f178e8 == PIN denoland/deno:2.5.6 sha256:3ea71953…f166aa == PIN denoland/deno:bin-2.5.6 sha256:2f5f9d65…d94c7 == PIN debian:bookworm-slim sha256:abd67ffc…29241 == PIN ``` The shape-first version matching in `version.ts` is genuinely careful — `sha256-…` rejected as a prefix, `bin-` never offered against plain, prerelease gated on the current tag. No complaints there. The checker half has three bugs that fire on **this** repository, not on a hypothetical one. --- ## Blockers ### 1. Issue identity is the name alone, and two dependencies here share a name The marker holds `name` + `latest`; `main.ts` matches on `name=<dep.name> `. But `Dependency` identity is kind + name + **version**, and the dry run finds two `denoland/deno` entries — `2.5.6` and `bin-2.5.6`. After the first run both issues carry `<!-- dep-check name=denoland/deno … -->`, so each dependency matches *both* issues and `decide` takes whichever open one comes first. Reproduced against the real `decide`, with two issues as the first run would leave them: ``` dep denoland/deno 2.5.6 -> {"do":"update","issue":102} # 102 is the bin issue dep denoland/deno bin-2.5.6 -> {"do":"skip","why":"already reported as bin-2.9.5"} ``` The bin issue gets rewritten with the plain image's title and body; the plain issue is orphaned and never touched again. Next Monday the marker says `2.9.5`, so it flips back. It ping-pongs one issue between two dependencies indefinitely, and the second issue is never closed even after the upgrade lands. The same collision runs through `resolve`: upgrading one variant closes the other variant's issue, and that close is then treated as a decision. Fix is small — put the declared version in the marker (`name=… from=… latest=…`) and match on name + version. ### 2. `latest === null` means two different things, and one of them closes issues `decide` reads a null `latest` as "the repository caught up". It is also what you get from a registry that answers `200 {"tags": []}`, from a truncated page, and from a trailing comment the regex read as junk (`# renovate: v6` yields `renovate:`). Reproduced: ``` pickLatest('v6', []) = null decide -> {"do":"resolve","issue":4} ``` That path comments *"no newer tag of that shape exists"* and closes the issue. Combined with the closed-issue rule — a decision, never reopened — a single bad answer permanently silences that dependency for that version. Nothing writes to the log to say it happened. `resolve` needs positive evidence: a non-empty tag list that contains the declared tag. Absence of a result is not the same as absence of a newer version, and this tool's own argument is that the distinction is what separates a checker from a checker that lies. ### 3. The 1000-tag cap is real and silent Measured against the live registry: ``` library/debian: 30 pages, 2976 tags # tool reads 1000, stops at "jessie-20200130" denoland/deno: returns exactly 1000 # cap hit, stops at "debian-1.24.3" ``` Both cap out. Docker Hub sorts lexically, so the numeric and `bin-` tags happen to land before the cutoff and today's answers are right **by accident**. Nothing warns when the cap is hit, so the first tag that sorts past it turns "up to date" into a guess with the same confident output. Cheapest correct fix: when the loop exits with `url` still set, return an `error` — `main` already skips those and reports them. --- ## Serious 4. **Actions never paginate at all** (`remote.ts`) — one request, `link` ignored. Counts today: checkout 68, login-action 43, build-push-action 74. Under the cap, but the same silent class as #3, and GitHub's `/tags` ordering is not a documented sort. 5. **No error handling around any forge write** (`main.ts`) — a single non-2xx from `create`/`update`/`comment`/`close` escapes `main()` and kills the run, leaving the remaining dependencies unprocessed. That directly contradicts the rule stated fifteen lines above it: *"one unreachable host must not cost the other twelve their issue."* The rule is right; it just is not applied to the write side. 6. **The entire mutating half is untested and has never executed.** `issues_test.ts` covers `decide`; `Forge` has no test. `deps.yml` has no `pull_request` or `push` trigger and Forgejo only schedules from the default branch, so nothing on this branch has exercised the workflow, the container, the token, or `checkout` inside a Deno image. The first run of `create`/`update`/`comment`/`close` will be unattended, against the live tracker — and it hits #1 on its second execution. `DEP_CHECK_FORGE_URL` / `DEP_CHECK_REPOSITORY` exist precisely so this can be rehearsed against a scratch repository; worth doing before merge, or adding a `Forge` test with an injected `fetch`. 7. **`docs/dependencies.md` opens by stating something that is not true of this repository** — that CI installs with `--frozen`. Nothing here passes it; the Dockerfile's install step is plain `deno install`, which resolves past a stale lock rather than failing on it. The description repeats the claim. In a document whose entire job is to be accurate about what is watched and what is not, this is the one line that is not. Adding `--frozen` to the Dockerfile is probably the better of the two fixes — same argument as the rest of this change, one layer up — but dropping the sentence also works. 8. **`deps.yml` has no `permissions:` block.** A pull request about narrowing supply-chain blast radius gives the job the runner's default full-repository token in order to file issues. `contents: read` + `issues: write` is one line, and it is the argument the rest of the change makes. --- ## Moderate 9. `update` fires on a change to `latest.tag` and never on a change to `dep.version`. Bump `v6` → `v6.5` while `v7` is still out and the issue keeps saying *"declared at **v6** and **v7** exists"*, with the title `v6 → v7`, indefinitely. Reproduced. 10. A SHA-pinned `uses:` with no trailing comment is dropped with no output at all — the exact failure this change exists to prevent, met with silence. 11. `.forgejo/workflows` and `docker/Dockerfile` are hardcoded single paths. The `COPY --from` hole this pull request just fixed was the same class of bug: a place nobody had listed. 12. `Forge.issues()` caps at 10 × 50 = 500 with `state=all` and stops silently; past that the tool stops finding its own issues and refiles duplicates. 13. `existing` is a single snapshot taken before the loop, so a create in one iteration is invisible to the next. Compounds #1. ## Minor - `workflowImagesFrom` matches `image:` at any nesting, including a `with:` input. - The `FROM` regex anchors on `$`, so `FROM x:1 # comment` is silently skipped. - `SHA` is 40 hex only; git's sha256 object format is 64. - `reports.sort` keys on `dep.name` alone, so the two `denoland/deno` rows swap order between runs. - *(outside this diff, flagged because this change is what makes it bite)* `deno task check` still only walks `packages/server/main.ts`, so a type error in the new `tools/dep-check` fails neither the image build nor the push to main — only the Monday job. That is what the "must fail here, not on Monday" comment in `deps.yml` is reaching for. --- ## Verdict Worth splitting. `06cd02f` — the pins — is correct, independently verified, and good to merge on its own. `e56579a` — the checker — should not be pointed at the live tracker until #1, #2 and #3 are fixed and the write path has been run once against a scratch repository. Everything above is reproducible; happy to hand over the probe scripts for #1, #2 and #3.
@ -0,0 +8,4 @@
# Monday morning, before the week's work starts. Forgejo only runs scheduled
# workflows from the default branch, so changes here take effect once merged.
on:
schedule:
Owner

No pull_request or push trigger, and Forgejo only runs scheduled workflows from the default branch — so this file, the container image, the token wiring, and actions/checkout running inside a Deno image (no git, so it takes the REST tarball path) have all never executed.

Everything verified in the description is the pure logic. The half that talks to the tracker is the half nothing has run.

No `pull_request` or `push` trigger, and Forgejo only runs scheduled workflows from the default branch — so this file, the container image, the token wiring, and `actions/checkout` running inside a Deno image (no `git`, so it takes the REST tarball path) have all never executed. Everything verified in the description is the pure logic. The half that talks to the tracker is the half nothing has run.
@ -0,0 +15,4 @@
workflow_dispatch:
jobs:
dependencies:
Owner

No permissions: block, so this job runs with the runner's default token scope in order to file issues.

A change about narrowing supply-chain blast radius handing out a full-repository write token is the one place the argument is not applied to itself. permissions: {contents: read, issues: write} is one line.

No `permissions:` block, so this job runs with the runner's default token scope in order to file issues. A change about narrowing supply-chain blast radius handing out a full-repository write token is the one place the argument is not applied to itself. `permissions: {contents: read, issues: write}` is one line.
@ -0,0 +26,4 @@
# the checker is part of the repository, so it is held to the same bar
# as the rest of it — a broken checker must fail here, not on Monday
- name: Typecheck
Owner

The comment says "a broken checker must fail here, not on Monday" — but this workflow only runs on Monday (or on manual dispatch), so "here" is Monday.

The check that would make the comment true is deno task check covering tools/dep-check, since that is what runs on every push to main via the image build. Right now a type error in this tool passes CI and surfaces a week later.

The comment says *"a broken checker must fail here, not on Monday"* — but this workflow only runs on Monday (or on manual dispatch), so "here" **is** Monday. The check that would make the comment true is `deno task check` covering `tools/dep-check`, since that is what runs on every push to main via the image build. Right now a type error in this tool passes CI and surfaces a week later.
@ -0,0 +4,4 @@
- **Deno packages** (`deno.json`, `deno.lock`) — `deno outdated` answers for
these, the lockfile keeps a resolve reproducible, and CI installs with
`--frozen` so a stale lock fails the build instead of being resolved past.
Owner

This is not true of this repository. Nothing here passes --frozen; the Dockerfile's install step is a plain deno install, which resolves past a stale lock rather than failing on it. The pull request description repeats the claim.

In a document whose only job is to be accurate about which layers are watched and which are not, this is the one line that is not. Adding --frozen to the Dockerfile is probably the better fix — it is the same argument as the rest of this change, one layer up — but dropping the sentence works too.

This is not true of this repository. Nothing here passes `--frozen`; the Dockerfile's install step is a plain `deno install`, which resolves past a stale lock rather than failing on it. The pull request description repeats the claim. In a document whose only job is to be accurate about which layers are watched and which are not, this is the one line that is not. Adding `--frozen` to the Dockerfile is probably the better fix — it is the same argument as the rest of this change, one layer up — but dropping the sentence works too.
@ -0,0 +49,4 @@
const name = ref.slice(0, at);
const rev = ref.slice(at + 1);
const version = SHA.test(rev) ? comment : rev;
if (!version) return;
Owner

A SHA-pinned uses: with no trailing comment is dropped here with no output whatsoever.

That is the precise failure this change exists to prevent — a hard pin with nothing watching it — and the tool's response is silence. It is also not a hypothetical: it is one forgotten comment away, in a repository that just pinned five actions.

Collect these and print them (? actions/checkout: pinned by SHA with no version comment), and count them in the summary.

A SHA-pinned `uses:` with no trailing comment is dropped here with no output whatsoever. That is the precise failure this change exists to prevent — a hard pin with nothing watching it — and the tool's response is silence. It is also not a hypothetical: it is one forgotten comment away, in a repository that just pinned five actions. Collect these and print them (`? actions/checkout: pinned by SHA with no version comment`), and count them in the summary.
@ -0,0 +63,4 @@
export function workflowImagesFrom(text: string, file: string): Dependency[] {
const out: Dependency[] = [];
text.split("\n").forEach((line, i) => {
const m = /^\s*image:\s*["']?([^"'\s]+)["']?\s*$/.exec(line);
Owner

This matches any line whose key is image:, at any nesting depth — a with: input to a scanning or build action would be picked up as a base image the repository depends on. Worth at least requiring the indentation of a container: child, or checking the preceding line.

This matches any line whose key is `image:`, at any nesting depth — a `with:` input to a scanning or build action would be picked up as a base image the repository depends on. Worth at least requiring the indentation of a `container:` child, or checking the preceding line.
@ -0,0 +91,4 @@
stages.has(ref.toLowerCase()) || /^\d+$/.test(ref);
text.split("\n").forEach((line, i) => {
const where = `${file}:${i + 1}`;
const from = /^\s*FROM\s+((?:--\S+\s+)*)(\S+)(?:\s+AS\s+(\S+))?\s*$/i.exec(
Owner

The $ anchor means FROM x:1 # comment does not match and is skipped silently. Same failure mode as the uncommented SHA pin above — a dependency that exists and is not watched, with nothing said about it.

The `$` anchor means `FROM x:1 # comment` does not match and is skipped silently. Same failure mode as the uncommented SHA pin above — a dependency that exists and is not watched, with nothing said about it.
@ -0,0 +154,4 @@
/** every dependency declared under `.forgejo/workflows` and in the Dockerfile */
export async function discover(root: string): Promise<Dependency[]> {
const found: Dependency[] = [];
const workflows = `${root}/.forgejo/workflows`;
Owner

.forgejo/workflows and docker/Dockerfile are hardcoded single paths.

The COPY --from=denoland/deno:bin-2.5.6 hole this pull request just found and fixed was exactly this class of bug: a place nobody had listed. A second Dockerfile added later is unwatched in the same way, and the tool will not say so.

`.forgejo/workflows` and `docker/Dockerfile` are hardcoded single paths. The `COPY --from=denoland/deno:bin-2.5.6` hole this pull request just found and fixed was exactly this class of bug: a place nobody had listed. A second Dockerfile added later is unwatched in the same way, and the tool will not say so.
@ -0,0 +43,4 @@
assertEquals(actionsFrom(pinned, "build.yml")[0].version, "v6");
});
Deno.test("a pinned action with no comment says nothing rather than guessing", () => {
Owner

This test locks in the silence rather than the behaviour that is wanted. "Says nothing rather than guessing" is right about not guessing a version, but an unwatched pin is the failure the whole change is built around — it should be reported, not merely not-guessed.

Assert on a warning or a second return channel, not on [].

This test locks in the silence rather than the behaviour that is wanted. "Says nothing rather than guessing" is right about not guessing a version, but an unwatched pin is the failure the whole change is built around — it should be reported, not merely not-guessed. Assert on a warning or a second return channel, not on `[]`.
@ -0,0 +72,4 @@
*/
async issues(): Promise<ExistingIssue[]> {
const out: ExistingIssue[] = [];
for (let page = 1; page <= 10; page++) {
Owner

Caps at 10 × 50 = 500 issues, state=all, and stops silently. Once this repository passes 500 issues the tool stops finding its own and starts refiling duplicates — the exact outcome the marker scheme exists to prevent.

Same silent-cap pattern as MAX_PAGES in remote.ts; worth fixing both the same way.

Caps at 10 × 50 = 500 issues, `state=all`, and stops silently. Once this repository passes 500 issues the tool stops finding its own and starts refiling duplicates — the exact outcome the marker scheme exists to prevent. Same silent-cap pattern as `MAX_PAGES` in `remote.ts`; worth fixing both the same way.
@ -0,0 +101,4 @@
return labels.find((l) => l.name === name)?.id ?? null;
}
async create(
Owner

This class is the entire mutating half of the tool and it has no test. issues_test.ts covers decide, which is the pure part.

Combined with deps.yml having no pull_request or push trigger, and Forgejo only running schedules from the default branch, nothing anywhere has executed create / update / comment / close. The first execution will be unattended, on a Monday, against the live tracker — and by then the name-collision blocker will already be in play.

DEP_CHECK_FORGE_URL / DEP_CHECK_REPOSITORY make a rehearsal against a scratch repository cheap. A Forge test with an injected fetch would be cheaper still.

This class is the entire mutating half of the tool and it has no test. `issues_test.ts` covers `decide`, which is the pure part. Combined with `deps.yml` having no `pull_request` or `push` trigger, and Forgejo only running schedules from the default branch, nothing anywhere has executed `create` / `update` / `comment` / `close`. The first execution will be unattended, on a Monday, against the live tracker — and by then the name-collision blocker will already be in play. `DEP_CHECK_FORGE_URL` / `DEP_CHECK_REPOSITORY` make a rehearsal against a scratch repository cheap. A `Forge` test with an injected `fetch` would be cheaper still.
@ -0,0 +36,4 @@
}
export function marker(name: string, latest: string): string {
return `<!-- dep-check name=${name} latest=${latest} -->`;
Owner

The marker is the identity, and it is missing the half that disambiguates. name=denoland/deno latest=2.9.5 and name=denoland/deno latest=bin-2.9.5 are the same dependency as far as main.ts is concerned — see the blocker there.

marker(name, from, latest) with from=<dep.version> fixes both this and the cross-variant resolve.

The marker is the identity, and it is missing the half that disambiguates. `name=denoland/deno latest=2.9.5` and `name=denoland/deno latest=bin-2.9.5` are the same dependency as far as `main.ts` is concerned — see the blocker there. `marker(name, from, latest)` with `from=<dep.version>` fixes both this and the cross-variant `resolve`.
@ -0,0 +91,4 @@
);
const open = mine.find((x) => x.issue.state === "open");
if (!latest) {
Owner

Blocker. A null latest is read as "the repository caught up", but it is also what a registry answering 200 {"tags": []} produces, and what a truncated tag list produces, and what a trailing comment the regex misread produces (# renovate: v6 yields renovate:).

pickLatest('v6', []) = null
decide -> {"do":"resolve","issue":4}

That comments "no newer tag of that shape exists" and closes the issue — and a closed issue is a decision that is never reopened for that version. One bad answer silences the dependency permanently, with nothing in the log to say so.

resolve should require positive evidence: a non-empty tag list containing the declared tag. Absence of a result is not absence of a newer version — which is this tool's own argument for why bookworm-slim is reported as "nothing to compare" rather than "up to date".

**Blocker.** A null `latest` is read as "the repository caught up", but it is also what a registry answering `200 {"tags": []}` produces, and what a truncated tag list produces, and what a trailing comment the regex misread produces (`# renovate: v6` yields `renovate:`). ``` pickLatest('v6', []) = null decide -> {"do":"resolve","issue":4} ``` That comments *"no newer tag of that shape exists"* and closes the issue — and a closed issue is a decision that is never reopened for that version. One bad answer silences the dependency permanently, with nothing in the log to say so. `resolve` should require positive evidence: a non-empty tag list containing the declared tag. Absence of a result is not absence of a newer version — which is this tool's own argument for why `bookworm-slim` is reported as "nothing to compare" rather than "up to date".
@ -0,0 +96,4 @@
? { do: "resolve", issue: open.issue.number }
: { do: "skip", why: "up to date" };
}
if (open) {
Owner

update triggers on a change to latest.tag and never on a change to dep.version. Bump v6v6.5 while v7 is still out and the issue keeps its original text forever:

decide({tag:"v7"}, [open latest=v7]) -> {"do":"skip","why":"already reported as v7"}
title stays: deps(action): actions/checkout v6 → v7
body stays:  `actions/checkout` is declared at **v6** and **v7** exists.

After a partial bump the issue states the wrong current version indefinitely.

`update` triggers on a change to `latest.tag` and never on a change to `dep.version`. Bump `v6` → `v6.5` while `v7` is still out and the issue keeps its original text forever: ``` decide({tag:"v7"}, [open latest=v7]) -> {"do":"skip","why":"already reported as v7"} title stays: deps(action): actions/checkout v6 → v7 body stays: `actions/checkout` is declared at **v6** and **v7** exists. ``` After a partial bump the issue states the wrong current version indefinitely.
@ -0,0 +98,4 @@
);
}
}
const existing: ExistingIssue[] = forge ? await forge.issues() : [];
Owner

One snapshot, taken before the loop, then never refreshed. An issue created in iteration N is invisible in iteration N+1 — which is part of why the two denoland/deno dependencies both file on the first run and then start fighting over one issue on the second.

One snapshot, taken before the loop, then never refreshed. An issue created in iteration N is invisible in iteration N+1 — which is part of why the two `denoland/deno` dependencies both file on the first run and then start fighting over one issue on the second.
@ -0,0 +104,4 @@
for (const { dep, latest, source, error } of reports) {
if (error) continue;
const what = `${dep.name} ${dep.version}`;
const mine = existing.filter((i) => i.body.includes(`name=${dep.name} `));
Owner

Blocker. Identity is the name alone, but Dependency identity is kind + name + version — and discover finds two denoland/deno entries in this repository (2.5.6 and bin-2.5.6). Both issues carry name=denoland/deno, so this filter returns both for each dependency and decide picks whichever open one is first.

Reproduced with two issues as the first run would leave them:

dep denoland/deno 2.5.6     -> {"do":"update","issue":102}   # 102 is the bin issue
dep denoland/deno bin-2.5.6 -> {"do":"skip","why":"already reported as bin-2.9.5"}

The bin issue is rewritten with the plain image's title and body; the plain issue is orphaned and never closed. Next week it flips back — one issue ping-ponging between two dependencies indefinitely.

Fix: carry the declared version in the marker and match on name + version.

**Blocker.** Identity is the name alone, but `Dependency` identity is kind + name + **version** — and `discover` finds two `denoland/deno` entries in this repository (`2.5.6` and `bin-2.5.6`). Both issues carry `name=denoland/deno`, so this filter returns both for each dependency and `decide` picks whichever open one is first. Reproduced with two issues as the first run would leave them: ``` dep denoland/deno 2.5.6 -> {"do":"update","issue":102} # 102 is the bin issue dep denoland/deno bin-2.5.6 -> {"do":"skip","why":"already reported as bin-2.9.5"} ``` The bin issue is rewritten with the plain image's title and body; the plain issue is orphaned and never closed. Next week it flips back — one issue ping-ponging between two dependencies indefinitely. Fix: carry the declared version in the marker and match on name + version.
@ -0,0 +141,4 @@
}
console.log(` + ${what}: ${latest!.tag} available, filing an issue`);
if (forge) {
const number = await forge.create(title, body, labels);
Owner

None of the forge writes in this loop are guarded. Any non-2xx from create / update / comment / close throws out of main(), so one failure halfway through leaves every remaining dependency unprocessed and the process exits on an unhandled rejection.

That is the rule stated at line 76 — "one unreachable host must not cost the other twelve their issue" — applied to reads but not to writes. Catch per dependency, count the failures, and return non-zero at the end.

None of the forge writes in this loop are guarded. Any non-2xx from `create` / `update` / `comment` / `close` throws out of `main()`, so one failure halfway through leaves every remaining dependency unprocessed and the process exits on an unhandled rejection. That is the rule stated at line 76 — *"one unreachable host must not cost the other twelve their issue"* — applied to reads but not to writes. Catch per dependency, count the failures, and return non-zero at the end.
@ -0,0 +89,4 @@
let token: string | null = null;
const tags: string[] = [];
try {
for (let page = 0; page < MAX_PAGES && url; page++) {
Owner

Blocker. This loop stops after 10 pages with no signal that it stopped early. Measured against the live registry:

library/debian: 30 pages, 2976 tags   # this reads 1000, stops at "jessie-20200130"
denoland/deno:  returns exactly 1000  # cap hit, stops at "debian-1.24.3"

Both cap out. Docker Hub sorts lexically, so the numeric and bin- tags land before the cutoff and today's answers are correct by luck. The first tag that sorts past it turns "up to date" into a guess delivered in the same confident tone.

When the loop exits with url still set, return fail(source, …)main already reports and skips errors rather than acting on them.

**Blocker.** This loop stops after 10 pages with no signal that it stopped early. Measured against the live registry: ``` library/debian: 30 pages, 2976 tags # this reads 1000, stops at "jessie-20200130" denoland/deno: returns exactly 1000 # cap hit, stops at "debian-1.24.3" ``` Both cap out. Docker Hub sorts lexically, so the numeric and `bin-` tags land before the cutoff and today's answers are correct by luck. The first tag that sorts past it turns "up to date" into a guess delivered in the same confident tone. When the loop exits with `url` still set, return `fail(source, …)` — `main` already reports and skips errors rather than acting on them.
@ -0,0 +134,4 @@
: `https://${host}/api/v1/repos/${name}/tags?limit=100`;
}
export async function listActionTags(
Owner

Actions are not paginated at all — one request at ?per_page=100 / ?limit=100, and the link header that nextLink already knows how to read is ignored here.

Counts today: actions/checkout 68, docker/login-action 43, docker/build-push-action 74. Under the cap, but the same silent-truncation class as the image path, and GitHub's /tags ordering is not a documented sort — so the newest tag being in the first page is not something the code is entitled to assume.

Actions are not paginated at all — one request at `?per_page=100` / `?limit=100`, and the `link` header that `nextLink` already knows how to read is ignored here. Counts today: `actions/checkout` 68, `docker/login-action` 43, `docker/build-push-action` 74. Under the cap, but the same silent-truncation class as the image path, and GitHub's `/tags` ordering is not a documented sort — so the newest tag being in the first page is not something the code is entitled to assume.
fix(dep-check): stop the checker acting on answers it never got
Some checks failed
Dependency Check / dependencies (pull_request) Failing after 15s
b2ade0a41b
Review found three ways this filed, rewrote or closed issues on the strength
of something it had not actually established. All three fire on this
repository rather than on a hypothetical one.

**Issue identity was the name alone.** `denoland/deno:2.5.6` and
`denoland/deno:bin-2.5.6` are two dependencies sharing a name, so after the
first run each matched both issues and `decide` took whichever came first:
one issue rewritten with the other's text, the second orphaned, the pair
flipping every week. Identity is now kind + name + tag SHAPE, which is stable
across both versions moving — the declared version cannot be the key either,
or bumping v6 to v6.5 would orphan the issue and file a duplicate.

**A null `latest` meant two different things and one of them closed issues.**
It is "nothing newer exists", and it is also what an empty tag list, a
truncated page and a misread version comment produce. Closing on that posts
"no newer tag of that shape exists" and shuts the issue, and a closed issue
is a decision that is never reopened — so one bad answer silenced a
dependency permanently. Resolving now requires positive evidence: a complete
tag list that contains the declared tag.

**Tag lists were being truncated in silence**, which turns "up to date" into
a guess delivered in the same confident tone. Measured: the ten-page ceiling
cut `library/debian` at 1000 of 2976 tags and `denoland/deno` at 1000 of
1751, and the action lookup did not paginate at all, reading 50 of
`actions/checkout`'s 68 because Forgejo caps a requested limit=100 at 50. The
ceiling is now 60 pages, actions follow `link` like images already did, and
reaching the end without exhausting the list is an ERROR rather than a short
answer. All four now read complete: 68, 74, 2976, 1751.

Also from the review:

- Forge writes are wrapped per dependency. The read side already said "one
  unreachable host must not cost the other twelve their issue"; the write
  side was the one place that rule was not applied, and any non-2xx killed
  the run mid-way.
- `Forge` has tests, with an injected `fetch`. It is the half that mutates a
  real tracker unattended, and it had never executed.
- `deps.yml` runs the whole thing with `--dry-run` on every pull request, so
  the workflow, container, checkout and entry point are exercised on the way
  in rather than for the first time on a Monday.
- `permissions: {contents: read, issues: write}`. A change about narrowing
  what a job can reach was handing this one the default full-repository
  token.
- A pin nothing can compare — `uses: …@<sha>` with no `# tag` comment, an
  untagged `FROM` — is reported and reddens the run instead of being dropped.
  That is the exact failure this tool exists to catch; meeting it with
  silence made the tool complicit in it.
- Issues are discovered by walking the tree for Dockerfiles and workflow
  directories rather than from two hardcoded paths. The `COPY --from` hole
  found yesterday was that same class of bug.
- `Forge.issues()` pages to the end instead of stopping at 500 and quietly
  refiling duplicates; the issue snapshot is kept current as the loop writes;
  an issue is rewritten when the DECLARED version moves, not only the
  available one; `image:` only counts under `container:`; `FROM x:1 # c` is
  no longer skipped by the `$` anchor; a 64-hex SHA is a SHA.
- `docker/Dockerfile` gains `deno install --frozen`, which `docs/dependencies.md`
  already claimed CI did. The claim is now true rather than deleted — same
  argument as the rest of this change, one layer up.
- `deno task check` covers `tools/dep-check`, so a type error here fails the
  image build rather than surfacing a week later in the Monday job.

Verified under the pinned CI toolchain (`denoland/deno:2.5.6`, via the digest
this repository pins) rather than the local Deno: fmt and lint clean, 44
tests pass, `deno check` clean on both entrypoints, dry run reports all seven
dependencies with both deno variants now distinct.

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

Pushed b2ade0a. All thirteen numbered items plus the minors are addressed; nothing declined. The three blockers were real and I reproduced each before fixing it.

Thank you for measuring rather than reasoning about #3 — that one was wrong in a way I could not have argued my way to.

Blockers

1. Name-only identity. Correct, and worse than a collision: the two denoland/deno entries would have rewritten each other's issue every week and orphaned one permanently. Your suggestion was name + from, but that trades this bug for #9's — bump v6 to v6.5 and the key changes, orphaning the issue and filing a duplicate. What is actually stable is the tag shape, which version.ts already computes for the matching rule: identity is now kind:name:shape, so image:denoland/deno:~ and image:denoland/deno:bin-~ are distinct and neither moves when a number does. from is in the marker as well, but as data (see #9), not as identity.

2. latest === null conflated. Correct, and the consequence you named — one bad answer permanently silencing a dependency, because the close is then treated as a decision — is the worst failure mode in the tool. decide now takes settled, which is the caller's answer to "do we hold a complete tag list containing the declared tag". Only that closes an issue. Everything else skips with a reason.

3. The 1000-tag cap. Real, and my fix is different from the one you proposed because the measurement pointed elsewhere. Erroring at the cap would have made debian and denoland/deno permanently unanswerable, since both exceed it. Following the pagination to the end instead:

before → after
actions/checkout          50 → 68     (Forgejo caps limit=100 at 50/page; no pagination at all)
docker/build-push-action  74 → 74
library/debian          1000 → 2976   (30 pages)
denoland/deno           1000 → 1751   (18 pages)

Both registries do serve the whole list; the ceiling was just too low. It is 60 pages now, actions follow link exactly as images already did, and reaching the ceiling is an error rather than a short answer — which is the part of your comment that mattered most. Your "correct by accident" reading was exactly right: lexical sort was doing the work.

Serious

4. Actions paginate. Same nextLink the image path used.

5. Every forge write is wrapped per dependency, counted, and the run exits non-zero. You were right that the rule was stated fifteen lines above the code that broke it.

6. Two of the three things you asked for, and one I cannot do from here honestly:

  • forge_test.ts — nine tests against an injected fetch: auth header, URL shape, request bodies for all four writes, non-2xx surfacing as an error, pagination to the first short page, and the refusal to decide from a truncated issue list.
  • deps.yml now has a pull_request trigger running --dry-run, so the workflow, the container, checkout inside a Deno image and the entry point all execute on the way in. This PR is the first exercise of it — if it goes red, that is the review working.
  • The write path still has not touched a real tracker. A scratch-repository rehearsal is the right last step and I would rather you say when, since it means creating and then deleting a repository on the instance. DEP_CHECK_FORGE_URL / DEP_CHECK_REPOSITORY are there for it.

7. Fixed by making the claim true: docker/Dockerfile now runs deno install --frozen. Same argument as the rest of the change, one layer up — and it is byte-identical to the line #10 adds, so the two merge cleanly.

8. permissions: {contents: read, issues: write}. One caveat written into the file rather than glossed: enforcement depends on the runner honouring permissions, which I have not verified on this instance. Stating it costs nothing and is the correct declaration either way, but I am not claiming a guarantee I have not tested.

Moderate and minor

9. An issue is rewritten when the declared version moves, not only the available one — both halves of the sentence it makes have to stay true. Test covers the partial bump.
10. A SHA pin with no comment now comes back in an unwatched channel, is printed, and reddens the run. So does an untagged FROM. You were right that the test locked in the silence; it now asserts the report.
11. Discovery walks the tree for Dockerfile* and both .forgejo/workflows and .github/workflows.
12. Forge.issues() pages to the first short page with no round-number ceiling, and throws rather than deciding from a partial list.
13. The snapshot is kept current as the loop writes.
Minors: image: only counts under container: (inline form handled too); the FROM regex allows a trailing comment; 64-hex SHAs are SHAs; sort breaks ties on version.
deno task check now covers tools/dep-check/main.ts, so a type error here fails the image build rather than waiting for Monday. That also makes the comment in deps.yml true, which it was not.

Verification

Run under the pinned CI toolchaindenoland/deno:2.5.6 by the digest this PR pins — rather than my local Deno, which is 2.9.4:

deno fmt --check   11 files clean
deno lint          10 files clean
deno check         packages/server/main.ts + tools/dep-check/main.ts clean
deno test          44 passed, 0 failed   (was 26)
dry run            7 dependencies, both deno variants distinct, exit 0

Worth flagging since it bit me: deno task check is red on main under Deno 2.9.4 — two Timeout-vs-number errors in packages/server that do not occur under 2.5.6. Not mine, and #10 fixes them properly. I mention it only because adding an entrypoint to that task makes it look like this PR broke it.

On splitting

You are right that 06cd02f stands alone and e56579a did not. I would rather not split now — the checker is the thing that makes the pins maintainable, and landing pins without it recreates the staleness problem that stopped anyone pinning in the first place. If you still want them apart after this pass, say so and I will reorder into two PRs with the pins first.

Pushed `b2ade0a`. All thirteen numbered items plus the minors are addressed; nothing declined. The three blockers were real and I reproduced each before fixing it. Thank you for measuring rather than reasoning about #3 — that one was wrong in a way I could not have argued my way to. ## Blockers **1. Name-only identity.** Correct, and worse than a collision: the two `denoland/deno` entries would have rewritten each other's issue every week and orphaned one permanently. Your suggestion was `name + from`, but that trades this bug for #9's — bump `v6` to `v6.5` and the key changes, orphaning the issue and filing a duplicate. What is actually stable is the tag *shape*, which `version.ts` already computes for the matching rule: identity is now `kind:name:shape`, so `image:denoland/deno:~` and `image:denoland/deno:bin-~` are distinct and neither moves when a number does. `from` is in the marker as well, but as data (see #9), not as identity. **2. `latest === null` conflated.** Correct, and the consequence you named — one bad answer permanently silencing a dependency, because the close is then treated as a decision — is the worst failure mode in the tool. `decide` now takes `settled`, which is the caller's answer to "do we hold a complete tag list containing the declared tag". Only that closes an issue. Everything else skips with a reason. **3. The 1000-tag cap.** Real, and my fix is different from the one you proposed because the measurement pointed elsewhere. Erroring at the cap would have made `debian` and `denoland/deno` permanently unanswerable, since both exceed it. Following the pagination to the end instead: ``` before → after actions/checkout 50 → 68 (Forgejo caps limit=100 at 50/page; no pagination at all) docker/build-push-action 74 → 74 library/debian 1000 → 2976 (30 pages) denoland/deno 1000 → 1751 (18 pages) ``` Both registries do serve the whole list; the ceiling was just too low. It is 60 pages now, actions follow `link` exactly as images already did, and **reaching the ceiling is an error rather than a short answer** — which is the part of your comment that mattered most. Your "correct by accident" reading was exactly right: lexical sort was doing the work. ## Serious **4.** Actions paginate. Same `nextLink` the image path used. **5.** Every forge write is wrapped per dependency, counted, and the run exits non-zero. You were right that the rule was stated fifteen lines above the code that broke it. **6.** Two of the three things you asked for, and one I cannot do from here honestly: - `forge_test.ts` — nine tests against an injected `fetch`: auth header, URL shape, request bodies for all four writes, non-2xx surfacing as an error, pagination to the first short page, and the refusal to decide from a truncated issue list. - `deps.yml` now has a `pull_request` trigger running `--dry-run`, so the workflow, the container, `checkout` inside a Deno image and the entry point all execute on the way in. **This PR is the first exercise of it** — if it goes red, that is the review working. - The write path still has not touched a real tracker. A scratch-repository rehearsal is the right last step and I would rather you say when, since it means creating and then deleting a repository on the instance. `DEP_CHECK_FORGE_URL` / `DEP_CHECK_REPOSITORY` are there for it. **7.** Fixed by making the claim true: `docker/Dockerfile` now runs `deno install --frozen`. Same argument as the rest of the change, one layer up — and it is byte-identical to the line #10 adds, so the two merge cleanly. **8.** `permissions: {contents: read, issues: write}`. One caveat written into the file rather than glossed: enforcement depends on the runner honouring `permissions`, which I have not verified on this instance. Stating it costs nothing and is the correct declaration either way, but I am not claiming a guarantee I have not tested. ## Moderate and minor **9.** An issue is rewritten when the declared version moves, not only the available one — both halves of the sentence it makes have to stay true. Test covers the partial bump. **10.** A SHA pin with no comment now comes back in an `unwatched` channel, is printed, and reddens the run. So does an untagged `FROM`. You were right that the test locked in the silence; it now asserts the report. **11.** Discovery walks the tree for `Dockerfile*` and both `.forgejo/workflows` and `.github/workflows`. **12.** `Forge.issues()` pages to the first short page with no round-number ceiling, and throws rather than deciding from a partial list. **13.** The snapshot is kept current as the loop writes. Minors: `image:` only counts under `container:` (inline form handled too); the `FROM` regex allows a trailing comment; 64-hex SHAs are SHAs; sort breaks ties on version. **`deno task check`** now covers `tools/dep-check/main.ts`, so a type error here fails the image build rather than waiting for Monday. That also makes the comment in `deps.yml` true, which it was not. ## Verification Run under the **pinned CI toolchain** — `denoland/deno:2.5.6` by the digest this PR pins — rather than my local Deno, which is 2.9.4: ``` deno fmt --check 11 files clean deno lint 10 files clean deno check packages/server/main.ts + tools/dep-check/main.ts clean deno test 44 passed, 0 failed (was 26) dry run 7 dependencies, both deno variants distinct, exit 0 ``` Worth flagging since it bit me: `deno task check` is **red on `main` under Deno 2.9.4** — two `Timeout`-vs-`number` errors in `packages/server` that do not occur under 2.5.6. Not mine, and #10 fixes them properly. I mention it only because adding an entrypoint to that task makes it look like this PR broke it. ## On splitting You are right that `06cd02f` stands alone and `e56579a` did not. I would rather not split now — the checker is the thing that makes the pins maintainable, and landing pins without it recreates the staleness problem that stopped anyone pinning in the first place. If you still want them apart after this pass, say so and I will reorder into two PRs with the pins first.
ci(deps): run the toolchain in the image, not the job in the image
Some checks failed
Dependency Check / dependencies (pull_request) Failing after 30s
5c7fa8222f
Two things the first CI run said, both correct.

`actions/checkout` is a Node action, and `denoland/deno:2.5.6` ships no node
— nor git, nor curl. A job whose `container:` is that image therefore cannot
check anything out: it dies with `exec: "node": executable file not found in
$PATH` before the first step does anything. The pin was not the problem and
is unchanged; the mistake was wrapping the job in the toolchain rather than
entering it. Deno is now invoked through `docker run` against the same
digest, which leaves the Node action on the runner where its interpreter
lives.

`permissions:` is not implemented by Forgejo — the run warns that it "is not
supported and will be ignored". A declared restriction that is not in force
is worse than an absent one, so it is gone, replaced by a comment naming the
lever that does exist: an Authorized Integration scoped to issues, supplied
as DEP_CHECK_TOKEN. Until that secret is set the job uses the runner's
default token, which docs/dependencies.md now says out loud rather than
implying otherwise.

Both steps run verbatim as written here: 44 tests pass and the dry run
reports all seven dependencies, exit 0.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ci(deps): copy the workspace in, since the runner's paths are not the host's
All checks were successful
Dependency Check / dependencies (pull_request) Successful in 1m9s
bd1c4c1d23
`-v "$PWD":/w` mounted an empty directory. The runner executes steps inside
its own container, so `$PWD` there is not a path the host docker daemon knows
— the mount silently succeeds against nothing and deno reports
`Import 'file:///w/tools/dep-check' failed, not found`.

`docker cp` streams through the daemon and needs no path in common, so the
workspace goes in that way instead. Both constraints that led here — a Node
action that cannot run inside the Deno image, and a bind mount that cannot
cross the runner's container boundary — are written down in .forgejo/deno.sh
rather than rediscovered next time.

The helper also forwards named environment variables (DENO_ENV) so the
issue-filing run can pass its token in, and carries the container's exit code
back out through `docker wait` — `docker start -a` alone would swallow it.

Exercised exactly as the workflow calls it: 44 tests pass, the dry run
reports seven dependencies and exits 0, a set variable is forwarded while an
unset one stays absent, and `Deno.exit(7)` comes back as 7.

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

CI is green at bd1c4c1 — run #428, 1m09s, pull_request. That is the first time this workflow has executed at all, and it took two failures to get there, both of which were mine and both worth writing down:

  • container: denoland/deno:2.5.6 cannot run actions/checkout. The image ships no node (nor git, nor curl), so the job died at exec: "node": executable file not found in $PATH before step one.
  • Replacing it with docker run -v "$PWD":/w mounted an empty directory. The runner executes steps inside its own container, so $PWD there is not a path the host daemon knows — the mount succeeds against nothing, surfacing as Import 'file:///w/tools/dep-check' failed, not found.

Both are now resolved in .forgejo/deno.sh: the job runs on the runner, each command enters the digest-pinned image, and the workspace is copied in with docker cp. Your #6 was right for a better reason than either of us gave — the workflow had never run, and it did not work.

I cannot mark these threads resolved; Forgejo's API has no endpoint for it. Here is the state of every item instead, so the thread can be closed out by eye.

Fixed as asked

# Where
4 actions never paginate remote.tstagsFromHost follows link; checkout now reads 68 tags, not 50
5 unguarded forge writes main.ts — try/catch per dependency, counted, run exits non-zero
7 --frozen claim untrue docker/Dockerfile — claim made true rather than deleted
9 update ignores dep.version issues.tsopen.mark.from === dep.version is part of the skip condition
10 silent SHA-pin drop discover.tsunwatched channel, printed, reddens the run
11 hardcoded paths discover.ts — walks for Dockerfile*, reads both workflow directories
12 500-issue cap forge.ts — pages to the first short page, throws rather than deciding from a partial list
13 stale snapshot main.tsexisting updated as the loop writes
minors image: only under container:; FROM x:1 # c no longer skipped; 64-hex SHAs; sort breaks ties on version; deno task check covers tools/dep-check

Fixed, but not the way you proposed

1, identity. You suggested name + from. That trades this bug for #9 — bump v6 to v6.5 and the key moves, orphaning the issue. Keyed on kind:name:shape instead, which is stable while either version moves. from is in the marker as data, not as identity.

3, the tag cap. Erroring at the cap would have made debian (2976 tags) and denoland/deno (1751) permanently unanswerable, since both exceed it. Raised the ceiling to 60 pages and made reaching it an error — the second half is the part your comment was actually about.

Cannot be done as asked

8, permissions:. The runner answered this one directly: "Job dependencies or its workflow has a permissions field, which is not supported in Forgejo and will be ignored. Use Authorized Integrations to grant capabilities to this job instead." I had added the block with a caveat that I had not verified enforcement; the honest resolution is that it is gone. A declared restriction that is not in force is worse than an absent one. The lever that does exist is documented: an Authorized Integration scoped to issues, supplied as DEP_CHECK_TOKEN. Until that secret exists this job runs with the runner's default token — stated in deps.yml and docs/dependencies.md rather than implied otherwise.

Still open

6, the write path has never touched a real tracker. Two thirds done: forge_test.ts covers it against an injected fetch, and the PR dry run now exercises the workflow, checkout and entry point for real. The live rehearsal against a scratch repository is the honest remaining gap — it means creating and deleting a repository on this instance, so I would rather @julian say when. DEP_CHECK_FORGE_URL / DEP_CHECK_REPOSITORY are there for it.


Unrelated to this PR but found while reading the runs: #10's verify job has the same container/checkout defect and has never passed. Runs 175–186 are twelve consecutive pull_request failures; every push-to-main run before them is green. Its stated purpose — "review has something to go on before main does" — is not being delivered, and every verification claim on that PR came from local runs. Flagged there rather than fixed, since it is your branch.

CI is green at `bd1c4c1` — run #428, 1m09s, `pull_request`. That is the first time this workflow has executed at all, and it took two failures to get there, both of which were mine and both worth writing down: - `container: denoland/deno:2.5.6` cannot run `actions/checkout`. The image ships no `node` (nor `git`, nor `curl`), so the job died at `exec: "node": executable file not found in $PATH` before step one. - Replacing it with `docker run -v "$PWD":/w` mounted an empty directory. The runner executes steps inside its own container, so `$PWD` there is not a path the host daemon knows — the mount succeeds against nothing, surfacing as `Import 'file:///w/tools/dep-check' failed, not found`. Both are now resolved in [.forgejo/deno.sh](../src/branch/ci/dependency-update-issues/.forgejo/deno.sh): the job runs on the runner, each command enters the digest-pinned image, and the workspace is copied in with `docker cp`. Your #6 was right for a better reason than either of us gave — the workflow had never run, and it did not work. I cannot mark these threads resolved; Forgejo's API has no endpoint for it. Here is the state of every item instead, so the thread can be closed out by eye. ### Fixed as asked | # | Where | | --- | --- | | 4 actions never paginate | `remote.ts` — `tagsFromHost` follows `link`; checkout now reads 68 tags, not 50 | | 5 unguarded forge writes | `main.ts` — try/catch per dependency, counted, run exits non-zero | | 7 `--frozen` claim untrue | `docker/Dockerfile` — claim made true rather than deleted | | 9 `update` ignores `dep.version` | `issues.ts` — `open.mark.from === dep.version` is part of the skip condition | | 10 silent SHA-pin drop | `discover.ts` — `unwatched` channel, printed, reddens the run | | 11 hardcoded paths | `discover.ts` — walks for `Dockerfile*`, reads both workflow directories | | 12 500-issue cap | `forge.ts` — pages to the first short page, throws rather than deciding from a partial list | | 13 stale snapshot | `main.ts` — `existing` updated as the loop writes | | minors | `image:` only under `container:`; `FROM x:1 # c` no longer skipped; 64-hex SHAs; sort breaks ties on version; `deno task check` covers `tools/dep-check` | ### Fixed, but not the way you proposed **1, identity.** You suggested `name + from`. That trades this bug for #9 — bump `v6` to `v6.5` and the key moves, orphaning the issue. Keyed on `kind:name:shape` instead, which is stable while either version moves. `from` is in the marker as data, not as identity. **3, the tag cap.** Erroring at the cap would have made `debian` (2976 tags) and `denoland/deno` (1751) permanently unanswerable, since both exceed it. Raised the ceiling to 60 pages **and** made reaching it an error — the second half is the part your comment was actually about. ### Cannot be done as asked **8, `permissions:`.** The runner answered this one directly: *"Job dependencies or its workflow has a permissions field, which is not supported in Forgejo and will be ignored. Use Authorized Integrations to grant capabilities to this job instead."* I had added the block with a caveat that I had not verified enforcement; the honest resolution is that it is gone. A declared restriction that is not in force is worse than an absent one. The lever that does exist is documented: an Authorized Integration scoped to issues, supplied as `DEP_CHECK_TOKEN`. **Until that secret exists this job runs with the runner's default token** — stated in `deps.yml` and `docs/dependencies.md` rather than implied otherwise. ### Still open **6, the write path has never touched a real tracker.** Two thirds done: `forge_test.ts` covers it against an injected `fetch`, and the PR dry run now exercises the workflow, checkout and entry point for real. The live rehearsal against a scratch repository is the honest remaining gap — it means creating and deleting a repository on this instance, so I would rather @julian say when. `DEP_CHECK_FORGE_URL` / `DEP_CHECK_REPOSITORY` are there for it. --- Unrelated to this PR but found while reading the runs: **#10's `verify` job has the same container/checkout defect** and has never passed. Runs 175–186 are twelve consecutive `pull_request` failures; every push-to-main run before them is green. Its stated purpose — "review has something to go on before main does" — is not being delivered, and every verification claim on that PR came from local runs. Flagged there rather than fixed, since it is your branch.
thisilike requested changes 2026-08-10 14:02:03 +02:00
Dismissed
thisilike left a comment

Second pass, at bd1c4c1 against base 1ae4844. Every inline comment is on a line this pull request adds.

Reproduced independently rather than read: 44 tests pass, dry run exits 0 with both denoland/deno variants distinct, deno lint clean over 10 files. forge_test.ts is the real thing — injected fetch, auth header, request bodies for all four writes, non-2xx rejecting, pagination stopping at the first short page, and the refusal to decide from an unbounded list. The settled flag, the 60-page ceiling that errors instead of truncating, link pagination for actions, per-dependency try/catch, the live existing snapshot, --frozen, and the from half of the marker all check out. Removing permissions: and quoting the runner's actual warning was the right call over keeping a declaration that is not enforced.

Three of the original thirteen are genuinely closed in a way I could verify by running the code. Two new problems arrived with the fixes, and one of them is worse than what it replaced.


Blockers

1. The shape key collides across declaration sites, and now it closes the issue

depKey is kind:name:shapeKey(version), and shapeKey deliberately erases the numbers. So two declarations of the same thing at different versions share one key:

depKey(action actions/checkout v6)     = action:actions/checkout:v~
depKey(action actions/checkout v7.0.1) = action:actions/checkout:v~   ← same

actions/checkout is declared in both build.yml:30 and deps.yml:34. It is also the subject of the first issue this tool will file. So the most likely next event in this repository is: someone acts on that issue, bumps build.yml, misses deps.yml. Then:

dep build.yml  v7.0.1 -> {"do":"resolve","issue":42}
dep deps.yml   v6     -> {"do":"skip","why":"already reported as v7.0.1"}

The run comments "no newer tag of that shape exists" and closes the issue on the strength of the site that was upgraded, while the other site is still on v6. And a closed issue is a decision that is never reopened for that version — so deps.yml stays on v6, permanently unwatched, with the tracker asserting it is current.

That is the same failure mode as blocker #2 last round (closing on an answer that does not mean what it is taken to mean), reached by a different route, and it now triggers on the ordinary success path rather than on a registry hiccup.

Keying on name + from was wrong for the reason you gave. But the fix is not a different key — it is that one key must not have two versions. Either fold same-key declarations into a single dependency whose where lists both sites and whose version is the oldest, or make a same-key/different-version pair a reported finding in its own right. The second is arguably the most useful sentence this tool could say: "actions/checkout is declared at v6 in one place and v7.0.1 in another." Nothing else in this repository is watching for that, which is the whole premise.

2. .forgejo/deno.sh pins a digest that nothing watches

IMAGE="denoland/deno:2.5.6@sha256:3ea71953…". Ran discovery against the tree:

7 dependencies, from .forgejo/workflows/*.yml and docker/Dockerfile
unwatched: []
.forgejo/deno.sh pins denoland/deno:2.5.6@sha256:3ea71953…
is it watched?  false

It is the only digest pin in the repository that discovery does not read, and it is in a file this pull request adds. This is COPY --from=denoland/deno:bin-2.5.6 verbatim — "a base image that appears in no FROM line, so nothing listed it and it would have stayed unpinned and unwatched" — which the description rightly presents as the thing pinning turned up. The fix for CI reintroduced it one directory over.

Worse than the original, because the tool now reports denoland/deno 2.5.6 as fully covered while one of its three declarations is invisible. Bump the Dockerfile and deno.sh silently stays behind, on a digest, forever.

Either teach discovery to read IMAGE=-shaped pins out of .forgejo/*.sh, or move the pin somewhere discovery already looks and have the script read it from there.


Security — confirm before merging

3. pull_request on a public repository, a self-hosted runner, and the host Docker socket

Adding the trigger was right, and #6 is better for it. But look at what the trigger now reaches: deps.yml runs on cth-ubuntu-latest and every step shells out to .forgejo/deno.sh, which runs docker create / docker cp / docker start against the host daemon. This repository is public (private: false).

A pull request from a fork supplies its own .forgejo/deno.sh, and the runner executes it. Access to the host Docker daemon is host root, on the same runner that build.yml uses with PACKAGE_TOKEN and DEPLOY_SSH_KEY. Forge secrets are withheld from fork pull requests, so the immediate reach is the runner rather than the secrets — but the runner is the thing that holds them on the next push to main.

I am flagging this rather than asserting it, because it turns entirely on whether this instance lets anyone fork. Worth confirming that before merge. If forks are possible, the usual gate is running the job only for same-repository pull requests:

if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository

Note this is not new in kind#10 adds on: pull_request to build.yml — but it is the first one that hands a pull-request-controlled script the Docker socket.


Moderate

  1. docker cp . "$id:/w" works by accident. With a directory source and an existing destination directory, docker cp copies the directory into the destination; with a non-existent destination it copies the contents. It does the second only because denoland/deno has no /w. A base image that ships one, or a change in docker cp's handling, nests the tree one level deeper and surfaces as exactly the Import 'file:///w/…' failed, not found the comment above it says was already solved once. docker cp ./. "$id:/w" states the intent instead of relying on the destination's absence.

  2. forge.label() is outside the guard that create/update/close now have. A token that may write issues but not read labels aborts the whole run before a single issue is filed — the failure mode #5 was about, in the one call that did not get wrapped. (issues() throwing is deliberate and correct; label() is not the same case, and the doc comment above it says as much: "should not fail the whole run on a label".)

  3. unwatched.length makes the run red. Right for the pull-request check. On the weekly run it means "somebody forgot a # v6 comment" and "the checker is broken" arrive as the same signal. Worth separating eventually; not worth blocking on.

  4. issues_test.ts covers two variants of one image having different identities, but not two versions of one variant. That is the hole blocker #1 lives in — the test that would have caught it is one line from one that exists.


Minor, and two things that are not your fault

  1. docs/modules.md (+256 non-whitespace lines) and docs/push-notifications.md (+82) are pure deno fmt reflow. Checked word by word: nothing changed but rewrapping, *em*_em_, table-separator padding, and reformatting of fenced code samples. 380 lines of unrelated diff a reviewer has to scan to establish they are noise. Worth its own chore(docs): deno fmt commit.

  2. Not yours, but relevant to that: deno fmt --check on this head under Deno 2.9.4 reports Found 81 not formatted files in 227. The formatter's output is toolchain-dependent, so the reflow above churns straight back the first time anyone formats on a newer Deno. Something to know before committing to formatted docs.

  3. Also not yours, and I want it on the record so it is not mistaken for a regression: deno task check is red under Deno 2.9.4 with the two Timeout-vs-number errors you flagged. I confirmed they are pre-existing — heartbeatTimer: number | null is unchanged at 1ae4844 — so adding tools/dep-check/main.ts to the task does not regress anything, and the image build runs under 2.5.6 where it passes. Your note about it was accurate.

  4. The host key. .forgejo/known_hosts parses as a well-formed ed25519 entry, SHA256:CBJYIn5cdGyK+GC3PFYeRpFya4Oz9vUfGwIsYyG06ZQ. Whether it is the right host is not something a reviewer can establish from the diff, and I am not going to open a connection to your production host to find out — so that verification stands on your two local checks, which is the correct place for it to stand. The change itself is right: replacing per-run ssh-keyscan with a pinned key, StrictHostKeyChecking=yes stated rather than defaulted, and one key rather than three. The -o UserKnownHostsFile=~/.ssh/known_hosts is redundant with the default but harmless.


Verdict

The three original blockers are properly dead, and #6 in particular is better than what I asked for — the workflow failing twice on the way in is worth more than the tests. But #1 and #2 are both regressions of the specific arguments this change is built on: acting on an answer that does not mean what it is taken to mean, and a pin with nothing watching it. Neither is hard to fix.

On splitting: I withdraw it. The checker is close enough that separating them would cost more than it buys.

Reproductions for #1 and #2 are two short scripts against your own exports; say the word and I will paste them.

Second pass, at `bd1c4c1` against base `1ae4844`. Every inline comment is on a line this pull request adds. Reproduced independently rather than read: **44 tests pass, dry run exits 0 with both `denoland/deno` variants distinct, `deno lint` clean over 10 files.** `forge_test.ts` is the real thing — injected `fetch`, auth header, request bodies for all four writes, non-2xx rejecting, pagination stopping at the first short page, and the refusal to decide from an unbounded list. The `settled` flag, the 60-page ceiling that errors instead of truncating, `link` pagination for actions, per-dependency try/catch, the live `existing` snapshot, `--frozen`, and the `from` half of the marker all check out. Removing `permissions:` and quoting the runner's actual warning was the right call over keeping a declaration that is not enforced. Three of the original thirteen are genuinely closed in a way I could verify by running the code. Two new problems arrived with the fixes, and one of them is worse than what it replaced. --- ## Blockers ### 1. The shape key collides across declaration sites, and now it *closes* the issue `depKey` is `kind:name:shapeKey(version)`, and `shapeKey` deliberately erases the numbers. So two declarations of the same thing at **different versions** share one key: ``` depKey(action actions/checkout v6) = action:actions/checkout:v~ depKey(action actions/checkout v7.0.1) = action:actions/checkout:v~ ← same ``` `actions/checkout` is declared in **both** `build.yml:30` and `deps.yml:34`. It is also the subject of the first issue this tool will file. So the most likely next event in this repository is: someone acts on that issue, bumps `build.yml`, misses `deps.yml`. Then: ``` dep build.yml v7.0.1 -> {"do":"resolve","issue":42} dep deps.yml v6 -> {"do":"skip","why":"already reported as v7.0.1"} ``` The run **comments "no newer tag of that shape exists" and closes the issue** on the strength of the site that was upgraded, while the other site is still on v6. And a closed issue is a decision that is never reopened for that version — so `deps.yml` stays on `v6`, permanently unwatched, with the tracker asserting it is current. That is the same failure mode as blocker #2 last round (closing on an answer that does not mean what it is taken to mean), reached by a different route, and it now triggers on the ordinary success path rather than on a registry hiccup. Keying on `name + from` was wrong for the reason you gave. But the fix is not a different key — it is that **one key must not have two versions**. Either fold same-key declarations into a single dependency whose `where` lists both sites and whose version is the oldest, or make a same-key/different-version pair a reported finding in its own right. The second is arguably the most useful sentence this tool could say: *"`actions/checkout` is declared at v6 in one place and v7.0.1 in another."* Nothing else in this repository is watching for that, which is the whole premise. ### 2. `.forgejo/deno.sh` pins a digest that nothing watches `IMAGE="denoland/deno:2.5.6@sha256:3ea71953…"`. Ran discovery against the tree: ``` 7 dependencies, from .forgejo/workflows/*.yml and docker/Dockerfile unwatched: [] .forgejo/deno.sh pins denoland/deno:2.5.6@sha256:3ea71953… is it watched? false ``` It is the only digest pin in the repository that discovery does not read, and it is in a file this pull request adds. This is `COPY --from=denoland/deno:bin-2.5.6` verbatim — *"a base image that appears in no `FROM` line, so nothing listed it and it would have stayed unpinned and unwatched"* — which the description rightly presents as the thing pinning turned up. The fix for CI reintroduced it one directory over. Worse than the original, because the tool now reports `denoland/deno 2.5.6` as fully covered while one of its three declarations is invisible. Bump the Dockerfile and `deno.sh` silently stays behind, on a digest, forever. Either teach discovery to read `IMAGE=`-shaped pins out of `.forgejo/*.sh`, or move the pin somewhere discovery already looks and have the script read it from there. --- ## Security — confirm before merging ### 3. `pull_request` on a public repository, a self-hosted runner, and the host Docker socket Adding the trigger was right, and #6 is better for it. But look at what the trigger now reaches: `deps.yml` runs on `cth-ubuntu-latest` and every step shells out to `.forgejo/deno.sh`, which runs `docker create` / `docker cp` / `docker start` against the **host** daemon. This repository is public (`private: false`). A pull request from a fork supplies its own `.forgejo/deno.sh`, and the runner executes it. Access to the host Docker daemon is host root, on the same runner that `build.yml` uses with `PACKAGE_TOKEN` and `DEPLOY_SSH_KEY`. Forge secrets are withheld from fork pull requests, so the immediate reach is the runner rather than the secrets — but the runner is the thing that holds them on the next push to main. I am flagging this rather than asserting it, because it turns entirely on whether this instance lets anyone fork. Worth confirming that before merge. If forks are possible, the usual gate is running the job only for same-repository pull requests: ```yaml if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ``` Note this is not new *in kind* — #10 adds `on: pull_request` to `build.yml` — but it is the first one that hands a pull-request-controlled script the Docker socket. --- ## Moderate 4. **`docker cp . "$id:/w"` works by accident.** With a directory source and an existing destination directory, `docker cp` copies the directory *into* the destination; with a non-existent destination it copies the contents. It does the second only because `denoland/deno` has no `/w`. A base image that ships one, or a change in `docker cp`'s handling, nests the tree one level deeper and surfaces as exactly the `Import 'file:///w/…' failed, not found` the comment above it says was already solved once. `docker cp ./. "$id:/w"` states the intent instead of relying on the destination's absence. 5. **`forge.label()` is outside the guard that `create`/`update`/`close` now have.** A token that may write issues but not read labels aborts the whole run before a single issue is filed — the failure mode #5 was about, in the one call that did not get wrapped. (`issues()` throwing is deliberate and correct; `label()` is not the same case, and the doc comment above it says as much: *"should not fail the whole run on a label"*.) 6. **`unwatched.length` makes the run red.** Right for the pull-request check. On the weekly run it means "somebody forgot a `# v6` comment" and "the checker is broken" arrive as the same signal. Worth separating eventually; not worth blocking on. 7. **`issues_test.ts` covers two *variants* of one image having different identities, but not two *versions* of one variant.** That is the hole blocker #1 lives in — the test that would have caught it is one line from one that exists. --- ## Minor, and two things that are not your fault 8. **`docs/modules.md` (+256 non-whitespace lines) and `docs/push-notifications.md` (+82) are pure `deno fmt` reflow.** Checked word by word: nothing changed but rewrapping, `*em*` → `_em_`, table-separator padding, and reformatting of fenced code samples. 380 lines of unrelated diff a reviewer has to scan to establish they are noise. Worth its own `chore(docs): deno fmt` commit. 9. **Not yours, but relevant to that:** `deno fmt --check` on this head under Deno 2.9.4 reports `Found 81 not formatted files in 227`. The formatter's output is toolchain-dependent, so the reflow above churns straight back the first time anyone formats on a newer Deno. Something to know before committing to formatted docs. 10. **Also not yours, and I want it on the record so it is not mistaken for a regression:** `deno task check` is red under Deno 2.9.4 with the two `Timeout`-vs-`number` errors you flagged. I confirmed they are pre-existing — `heartbeatTimer: number | null` is unchanged at `1ae4844` — so adding `tools/dep-check/main.ts` to the task does not regress anything, and the image build runs under 2.5.6 where it passes. Your note about it was accurate. 11. **The host key.** `.forgejo/known_hosts` parses as a well-formed ed25519 entry, `SHA256:CBJYIn5cdGyK+GC3PFYeRpFya4Oz9vUfGwIsYyG06ZQ`. Whether it is the *right* host is not something a reviewer can establish from the diff, and I am not going to open a connection to your production host to find out — so that verification stands on your two local checks, which is the correct place for it to stand. The change itself is right: replacing per-run `ssh-keyscan` with a pinned key, `StrictHostKeyChecking=yes` stated rather than defaulted, and one key rather than three. The `-o UserKnownHostsFile=~/.ssh/known_hosts` is redundant with the default but harmless. --- ## Verdict The three original blockers are properly dead, and #6 in particular is better than what I asked for — the workflow failing twice on the way in is worth more than the tests. But #1 and #2 are both regressions of the specific arguments this change is built on: acting on an answer that does not mean what it is taken to mean, and a pin with nothing watching it. Neither is hard to fix. On splitting: I withdraw it. The checker is close enough that separating them would cost more than it buys. Reproductions for #1 and #2 are two short scripts against your own exports; say the word and I will paste them.
@ -0,0 +18,4 @@
# Usage: .forgejo/deno.sh test --allow-read tools/dep-check/
set -eu
IMAGE="denoland/deno:2.5.6@sha256:3ea71953ff50e3ff15c377ead1a8521f624e2f43d27713675a8bed7b33f166aa"
Owner

Blocker. This digest is pinned and nothing watches it. Ran discovery against the tree:

7 dependencies, from .forgejo/workflows/*.yml and docker/Dockerfile
unwatched: []
.forgejo/deno.sh pins denoland/deno:2.5.6@sha256:3ea71953…
is it watched?  false

It is the only digest pin in the repository discovery does not read.

This is COPY --from=denoland/deno:bin-2.5.6 again — "a base image that appears in no FROM line, so nothing listed it and it would have stayed unpinned and unwatched", which the description presents as the find that justified the whole exercise. The CI fix reintroduced it one directory over.

Worse than the original: the tool now reports denoland/deno 2.5.6 as covered while one of its three declarations is invisible. Bump the Dockerfile and this stays behind, on a digest, indefinitely.

Either read IMAGE=-shaped pins out of .forgejo/*.sh, or move the pin to somewhere discovery already looks and have the script read it from there.

**Blocker.** This digest is pinned and nothing watches it. Ran discovery against the tree: ``` 7 dependencies, from .forgejo/workflows/*.yml and docker/Dockerfile unwatched: [] .forgejo/deno.sh pins denoland/deno:2.5.6@sha256:3ea71953… is it watched? false ``` It is the only digest pin in the repository discovery does not read. This is `COPY --from=denoland/deno:bin-2.5.6` again — *"a base image that appears in no FROM line, so nothing listed it and it would have stayed unpinned and unwatched"*, which the description presents as the find that justified the whole exercise. The CI fix reintroduced it one directory over. Worse than the original: the tool now reports `denoland/deno 2.5.6` as covered while one of its three declarations is invisible. Bump the Dockerfile and this stays behind, on a digest, indefinitely. Either read `IMAGE=`-shaped pins out of `.forgejo/*.sh`, or move the pin to somewhere discovery already looks and have the script read it from there.
@ -0,0 +31,4 @@
id=$(docker create -w /w $env_args "$IMAGE" deno "$@")
trap 'docker rm -f "$id" >/dev/null 2>&1 || true' EXIT
docker cp . "$id:/w" >/dev/null
Owner

This works by accident. docker cp copies a source directory into an existing destination directory, and copies its contents only when the destination does not exist. It does the second here purely because denoland/deno has no /w.

A base image that ships one — or a change in how docker cp resolves . — nests the workspace a level deeper and surfaces as exactly the Import 'file:///w/…' failed, not found the comment above says was already solved once.

docker cp ./. "$id:/w" states the intent rather than depending on the destination's absence.

This works by accident. `docker cp` copies a source directory *into* an existing destination directory, and copies its *contents* only when the destination does not exist. It does the second here purely because `denoland/deno` has no `/w`. A base image that ships one — or a change in how `docker cp` resolves `.` — nests the workspace a level deeper and surfaces as exactly the `Import 'file:///w/…' failed, not found` the comment above says was already solved once. `docker cp ./. "$id:/w"` states the intent rather than depending on the destination's absence.
@ -0,0 +20,4 @@
#
# checked against the key the host reports for itself over an already-trusted
# session (`ssh <host> cat /etc/ssh/ssh_host_ed25519_key.pub`).
192.168.0.3 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICgU0tgg/3hqPSKtwmnTyG+6DEQSVxygoMj5wXlJS998
Owner

Parses as a well-formed ed25519 entry, SHA256:CBJYIn5cdGyK+GC3PFYeRpFya4Oz9vUfGwIsYyG06ZQ.

Whether it is the right host is not establishable from a diff, and I am not going to open a connection to your production host to find out — so that rests on your two local checks, which is where it belongs. Worth saying explicitly rather than implying the pin was verified end to end by review.

The change itself is right: per-run ssh-keyscan replaced by a pinned key, StrictHostKeyChecking=yes stated rather than defaulted, and one key instead of three. The reasoning for ed25519-only is correct — extra host key types are extra ways for verification to pass after this one is replaced.

Parses as a well-formed ed25519 entry, `SHA256:CBJYIn5cdGyK+GC3PFYeRpFya4Oz9vUfGwIsYyG06ZQ`. Whether it is the *right* host is not establishable from a diff, and I am not going to open a connection to your production host to find out — so that rests on your two local checks, which is where it belongs. Worth saying explicitly rather than implying the pin was verified end to end by review. The change itself is right: per-run `ssh-keyscan` replaced by a pinned key, `StrictHostKeyChecking=yes` stated rather than defaulted, and one key instead of three. The reasoning for ed25519-only is correct — extra host key types are extra ways for verification to pass after this one is replaced.
@ -0,0 +17,4 @@
# Without this the workflow, the checkout and the tool's own entry point
# would first execute unattended on a Monday, against the live tracker —
# the one place where "it has never run" is least acceptable.
pull_request:
Owner

Security — worth confirming before this merges. Adding the trigger was right and #6 is better for it. But consider what it now reaches: this job runs on cth-ubuntu-latest and every step shells out to .forgejo/deno.sh, which drives the host Docker daemon (docker create / cp / start). This repository is public.

A fork's pull request supplies its own .forgejo/deno.sh and the runner executes it. Docker socket access is host root — on the same runner build.yml uses with PACKAGE_TOKEN and DEPLOY_SSH_KEY. Fork pull requests do not get secrets, so the immediate reach is the runner rather than the credentials; the runner is what holds them on the next push to main.

This turns on whether your instance lets anyone fork, which I cannot tell from here. If it does, the usual gate:

if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository

Not new in kind — #10 adds on: pull_request to build.yml — but this is the first one handing a pull-request-controlled script the Docker socket.

**Security — worth confirming before this merges.** Adding the trigger was right and #6 is better for it. But consider what it now reaches: this job runs on `cth-ubuntu-latest` and every step shells out to `.forgejo/deno.sh`, which drives the **host** Docker daemon (`docker create` / `cp` / `start`). This repository is public. A fork's pull request supplies its own `.forgejo/deno.sh` and the runner executes it. Docker socket access is host root — on the same runner `build.yml` uses with `PACKAGE_TOKEN` and `DEPLOY_SSH_KEY`. Fork pull requests do not get secrets, so the immediate reach is the runner rather than the credentials; the runner is what holds them on the next push to main. This turns on whether your instance lets anyone fork, which I cannot tell from here. If it does, the usual gate: ```yaml if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ``` Not new in kind — #10 adds `on: pull_request` to `build.yml` — but this is the first one handing a pull-request-controlled script the Docker socket.
@ -20,3 +20,3 @@
"build": "deno task build:shell && deno task build:shared && deno task build:modules",
"start": "deno run --allow-read --allow-write=data --allow-env --allow-net --allow-ffi --allow-run packages/server/main.ts",
"check": "deno check packages/server/main.ts",
"check": "deno check packages/server/main.ts tools/dep-check/main.ts",
Owner

Correct fix for the "fails on Monday" problem, and I want the consequence on the record so it is not mistaken for a regression from this change:

deno task check is red under Deno 2.9.4 with the two Timeout-vs-number errors you flagged. I confirmed they pre-date this branch — heartbeatTimer: number | null is unchanged at 1ae4844 — so adding this entry point does not regress anything, and the image build runs under 2.5.6 where it passes. Your note about it was accurate.

Correct fix for the "fails on Monday" problem, and I want the consequence on the record so it is not mistaken for a regression from this change: `deno task check` is red under Deno 2.9.4 with the two `Timeout`-vs-`number` errors you flagged. I confirmed they pre-date this branch — `heartbeatTimer: number | null` is unchanged at `1ae4844` — so adding this entry point does not regress anything, and the image build runs under 2.5.6 where it passes. Your note about it was accurate.
@ -50,3 +50,3 @@
// admin-only mutation
ctx.router.post("/things", ctx.auth.requireRole("admin"), (c) => c.json({ ok: true }));
ctx.router.post(
Owner

This file and docs/push-notifications.md are pure deno fmt reflow — I checked word by word, and nothing changed but rewrapping, *em*_em_, table-separator padding, and reformatting inside fenced code samples. 256 and 82 non-whitespace lines respectively.

No objection to the formatting; it is 380 lines of unrelated diff a reviewer has to read to establish they are noise. Own commit — chore(docs): deno fmt — keeps the pinning change reviewable.

Worth knowing before committing to this: deno fmt --check on this head under Deno 2.9.4 reports Found 81 not formatted files in 227. The formatter's output is toolchain-dependent, so this reflow churns straight back the first time anyone formats on a newer Deno.

This file and `docs/push-notifications.md` are pure `deno fmt` reflow — I checked word by word, and nothing changed but rewrapping, `*em*` → `_em_`, table-separator padding, and reformatting inside fenced code samples. 256 and 82 non-whitespace lines respectively. No objection to the formatting; it is 380 lines of unrelated diff a reviewer has to read to establish they are noise. Own commit — `chore(docs): deno fmt` — keeps the pinning change reviewable. Worth knowing before committing to this: `deno fmt --check` on this head under Deno 2.9.4 reports `Found 81 not formatted files in 227`. The formatter's output is toolchain-dependent, so this reflow churns straight back the first time anyone formats on a newer Deno.
@ -0,0 +225,4 @@
".claude",
]);
const WORKFLOW_DIRS = [".forgejo/workflows", ".github/workflows"];
Owner

Discovery reads workflow YAML and Dockerfile*. .forgejo/deno.sh now pins a digest too, and this list is why it goes unseen — see the blocker on that file.

The walk itself is a real improvement over the two hardcoded paths, and skipping .claude is a good catch given worktrees live there.

Discovery reads workflow YAML and `Dockerfile*`. `.forgejo/deno.sh` now pins a digest too, and this list is why it goes unseen — see the blocker on that file. The walk itself is a real improvement over the two hardcoded paths, and skipping `.claude` is a good catch given worktrees live there.
@ -0,0 +40,4 @@
* its own. Not the declared version either: bumping `v6` to `v6.5` would
* orphan the issue and file a duplicate. The shape is what stays put.
*/
export const depKey = (dep: Dependency): string =>
Owner

Blocker — this is where the close comes from. shapeKey erases the numbers on purpose, so two declarations of the same thing at different versions share one key:

depKey(action actions/checkout v6)     = action:actions/checkout:v~
depKey(action actions/checkout v7.0.1) = action:actions/checkout:v~

actions/checkout is declared in both build.yml:30 and deps.yml:34, and it is the subject of the first issue this tool will file. So the likely next event is a partial bump — build.yml moved, deps.yml missed:

dep build.yml  v7.0.1 -> {"do":"resolve","issue":42}
dep deps.yml   v6     -> {"do":"skip","why":"already reported as v7.0.1"}

The run comments "no newer tag of that shape exists" and closes the issue, while a site is still on v6 — and a closed issue is never reopened for that version. deps.yml is then stale, unwatched, with the tracker asserting it is current.

name + from was wrong for the reason you gave. The fix is not a different key: one key must not hold two versions. Fold same-key declarations into one dependency (oldest version, both sites in where), or report the pair as a finding — "declared at v6 here and v7.0.1 there" is arguably the most valuable thing this tool could say, and nothing else watches for it.

**Blocker — this is where the close comes from.** `shapeKey` erases the numbers on purpose, so two declarations of the same thing at different versions share one key: ``` depKey(action actions/checkout v6) = action:actions/checkout:v~ depKey(action actions/checkout v7.0.1) = action:actions/checkout:v~ ``` `actions/checkout` is declared in both `build.yml:30` and `deps.yml:34`, and it is the subject of the first issue this tool will file. So the likely next event is a partial bump — `build.yml` moved, `deps.yml` missed: ``` dep build.yml v7.0.1 -> {"do":"resolve","issue":42} dep deps.yml v6 -> {"do":"skip","why":"already reported as v7.0.1"} ``` The run comments *"no newer tag of that shape exists"* and closes the issue, while a site is still on v6 — and a closed issue is never reopened for that version. `deps.yml` is then stale, unwatched, with the tracker asserting it is current. `name + from` was wrong for the reason you gave. The fix is not a different key: **one key must not hold two versions.** Fold same-key declarations into one dependency (oldest version, both sites in `where`), or report the pair as a finding — *"declared at v6 here and v7.0.1 there"* is arguably the most valuable thing this tool could say, and nothing else watches for it.
@ -0,0 +46,4 @@
assertEquals(issueTitle(DEP, V7), "deps(action): actions/checkout v6 → v7");
});
Deno.test("two variants of one image have different identities", () => {
Owner

This covers two variants of one image having different identities, which is the bug from last round and is properly dead.

The hole blocker #1 lives in is one line away: two versions of one variant — actions/checkout at v6 in deps.yml and v7.0.1 in build.yml — which share a key and make the run close the issue while a site is still stale.

This covers two *variants* of one image having different identities, which is the bug from last round and is properly dead. The hole blocker #1 lives in is one line away: two *versions* of one variant — `actions/checkout` at `v6` in `deps.yml` and `v7.0.1` in `build.yml` — which share a key and make the run close the issue while a site is still stale.
@ -0,0 +112,4 @@
return 2;
}
forge = new Forge(cfg);
const id = await forge.label(LABEL);
Owner

label() is the one forge call left outside the guard that create / update / comment / close now have. A token that may write issues but not read labels aborts the run here, before anything is filed — which is the failure mode #5 was about.

The doc comment on Forge.label already states the intent: "a token that may only write issues should not fail the whole run on a label". It just is not what the code does. Catching around this and falling through to labels = [] finishes the thought.

(issues() throwing is deliberate and correct — deciding from a partial issue list is the thing that refiles duplicates.)

`label()` is the one forge call left outside the guard that `create` / `update` / `comment` / `close` now have. A token that may write issues but not read labels aborts the run here, before anything is filed — which is the failure mode #5 was about. The doc comment on `Forge.label` already states the intent: *"a token that may only write issues should not fail the whole run on a label"*. It just is not what the code does. Catching around this and falling through to `labels = []` finishes the thought. (`issues()` throwing is deliberate and correct — deciding from a partial issue list is the thing that refiles duplicates.)
@ -0,0 +193,4 @@
// Red when something was actually wrong: a write that failed, a dependency
// nothing can watch, or every single lookup failing (one is noise, all of
// them is a broken checker).
return failed || unwatched.length || unreachable === reports.length ? 1 : 0;
Owner

unwatched.length reddening the run is right for the pull-request check — a pin nobody can compare should stop the merge.

On the weekly run it means "somebody forgot a # v6 comment" and "the checker itself is broken" arrive as the same red. Worth separating eventually; not worth blocking on.

`unwatched.length` reddening the run is right for the pull-request check — a pin nobody can compare should stop the merge. On the weekly run it means "somebody forgot a `# v6` comment" and "the checker itself is broken" arrive as the same red. Worth separating eventually; not worth blocking on.
@ -0,0 +64,4 @@
*
* An unversioned tag has no shape to speak of, so it stands for itself.
*/
export function shapeKey(version: string): string {
Owner

The doc comment argues the shape is what stays put while the numbers move, and that is right for one declaration site. It stops being true with two: 2.5.6 and 2.9.5 are the same shape and must not be the same issue.

Whichever way #1 is resolved, this function is fine — the collision belongs to depKey's use of it, not to the definition of a shape.

The doc comment argues the shape is what stays put while the numbers move, and that is right for one declaration site. It stops being true with two: `2.5.6` and `2.9.5` are the same shape and must not be the same issue. Whichever way #1 is resolved, this function is fine — the collision belongs to `depKey`'s use of it, not to the definition of a shape.
modules.md and push-notifications.md were rewrapped by a repository-wide
`deno fmt`: 380 lines of pure reflow — rewrapping, *em* -> _em_, table
padding — in a pull request about pinning. Nothing in it changed a word, and
it is 380 lines a reviewer has to read to establish that.

Nothing checks formatting in CI, and `deno fmt` output is toolchain-dependent
(2.9.4 reports 81 unformatted files on this tree), so committing the reflow
here would also churn straight back the first time anyone formats on a newer
Deno. Reverted to base; formatting the docs is its own decision.
forge.label() was the one forge call left outside the guard create/update/
comment/close have. A token allowed to write issues but not to read labels —
an ordinary Authorized Integration scope — threw here, before a single issue
was filed, and took the whole run with it.

That is the failure the per-dependency guard was added to prevent, in the one
call that did not get it, and the doc comment on Forge.label already stated
the intent: a token that may only write issues should not fail the whole run
on a label. Now it does not; the run files without the label and says so.

Forge.issues() stays unguarded on purpose: deciding from a partial issue list
is what refiles duplicates, so that one has to be fatal.
depKey erases the numbers on purpose, so two declarations of one thing at
different versions shared a key. actions/checkout is declared in both
build.yml and deps.yml, and it is the subject of the first issue this tool
will file — so the likely next event was a partial bump: build.yml moved to
v7.0.1, deps.yml missed. The run then answered for the upgraded site,
commented "no newer tag of that shape exists" and closed the issue while the
other site was still on v6. A closed issue is never reopened for that version,
so deps.yml would have stayed stale, unwatched, with the tracker asserting it
was current.

The key was not the problem — one key holding two versions was. Declarations
now fold by issue identity into a single Dependency carrying every site, and
the version it reports is the OLDEST of them. The issue stays open until every
site has moved, which is when it is actually true.

The disagreement is itself worth saying, and nothing else here watches for it:
when sites differ the issue lists which version is where, and the run logs it
on a dry run too. dedupe() becomes fold(), Declaration is what one line says
and Dependency is what one issue is about, and depKey moves next to the fold
so the marker key and the fold key cannot drift apart.

Verified against the real tree: checkout folds to one dependency with both
sites; with build.yml patched to v7.0.1 the issue is about v6 and reports
"v7.0.1 build.yml:30, v6 deps.yml:45". 48 tests pass, including the two-
versions-of-one-variant case the previous round's test was one line from.
Two fixes to the same script, both about it doing the thing this change
exists to prevent.

The IMAGE= digest was a pin nothing watched. discover reads workflow YAML and
Dockerfiles, so it never saw this file — which is COPY --from=denoland/deno:
bin-2.5.6 verbatim, the base image in no FROM line that the pinning turned up,
reintroduced one directory over. Worse, because the tool then reported
denoland/deno 2.5.6 as covered while one of its declarations was invisible.
Rather than teach discover a new file format, the pin is gone: the image is
read from the Dockerfile's build-stage FROM, which is already watched. One
declaration, and CI now provably runs the toolchain the image is built with.

docker cp . worked by accident. With a directory source it copies the
directory INTO an existing destination and its contents into a missing one —
it did the second only because this image has no /w. Reproduced against the
pinned image with a destination that does exist:

  docker cp $dir  $id:/tmp  -> /tmp/w
  docker cp $dir/. $id:/tmp -> /tmp/marker.txt

A base image shipping /w would have nested the workspace and surfaced as the
Import 'file:///w/…' failed, not found that the comment above says was already
solved once. ./. asks for the contents either way.

Verified: sh .forgejo/deno.sh test --allow-read tools/dep-check/ -> 48 passed,
and no file under .forgejo carries a sha256 pin any more.
Adding the pull_request trigger was right, but it hands a pull-request-
controlled script the host docker daemon. Every step shells out to
.forgejo/deno.sh, which runs docker create/cp/start against the HOST daemon;
daemon access is root on the runner, and that runner is the one build.yml uses
with PACKAGE_TOKEN and DEPLOY_SSH_KEY. A fork's pull request supplies its own
copy of the script and the runner executes it.

Checked rather than assumed: the repository is public, /api/v1/settings/
repository reports forks_disabled false, and /user/sign_up serves a
registration form — so the trigger is reachable by anyone with an account.
Secrets are withheld from fork runs, so the immediate reach is the runner, not
the credentials; the runner is what holds them on the next push to main.

Restricted to same-repository pull requests. Scheduled and dispatched runs are
unaffected.
docs(dependencies): the fold, the single toolchain pin, and the fork gate
All checks were successful
Dependency Check / dependencies (pull_request) Successful in 1m13s
ab305fcf32
The pinned-today list said deps.yml pins the Deno image it runs in, which is
no longer true and was the kind of line that makes a document about what is
watched the one thing that is not.
Author
Owner

Pushed six commits at ab305fc. Both blockers fixed, the security question answered with a check rather than a guess, and the three moderates that were fixable are fixed. One is deferred, with the reason; nothing is disputed.

You were right about the shape of #1 in a way I want to name before the fix: both blockers are the same mistake I said this change existed to prevent, made one layer out from where I was looking. Acting on an answer that does not mean what it is taken to mean, and a pin with nothing watching it.


1. One key must not hold two versions — c069a7c

Fixed by folding, not by re-keying. dedupe() is now fold(), and it folds on the issue identity rather than on kind+name+version, producing one Dependency that carries every declaration in sites and reports the oldest version among them.

That makes the partial bump you described a non-event. The issue is about the site still on v6, so it stays open — and correct — until every site has moved:

after a partial bump (build.yml -> v7.0.1, deps.yml missed):
  issue is about v6; sites = ["v7.0.1 build.yml:30", "v6 deps.yml:45"]
  decide -> {"do":"skip","why":"already reported as v7.0.1"}
and once deps.yml moves too:
  decide -> {"do":"resolve","issue":42}

Which does mean the title reads v6 → v7.0.1 for as long as one site lags, while another already sits on v7.0.1. That is the intended reading: the issue is not "a newer version exists", it is "this repository is not on it yet", and it should not stop saying so while that is true.

I took your second suggestion as well, because you were right that it is the most useful sentence available. When sites disagree the issue says which version is where —

  • declared at different versions: v7.0.1 in build.yml:30, v6 in deps.yml:45 — this issue is about the oldest, and stays open until every one of them has moved

— and the run logs it, so a --dry-run surfaces it too. Nothing else here watches for that.

Two definitions had drifted apart and are now one: depKey lives in discover.ts next to the fold that needs it and is re-exported from issues.ts. The marker key and the fold key being the same key is load-bearing; two definitions of it was how this got in. Declaration is what one line says, Dependency is what one issue is about.

Your inline note on shapeKey was right too — I left it alone, the collision was depKey's use of it.

2. .forgejo/deno.sh563ecf3

Removed the pin rather than teaching discovery to read shell scripts. The image comes out of the Dockerfile's build-stage FROM, which is already watched:

IMAGE=$(sed -n 's/^FROM  *\(denoland\/deno:[^ ]*\).*/\1/p' "$DOCKERFILE" | head -n 1)

One declaration instead of two, and it is one the checker reads. It also closes a gap I had only asserted: the script's own comment says the toolchain must be the same image the Dockerfile builds with, and now that is enforced rather than maintained by hand. Empty IMAGE exits 1 with a message.

.forgejo files carrying a sha256 pin: []
$ sh .forgejo/deno.sh test --allow-read tools/dep-check/   ->  48 passed, 0 failed

3. Fork pull requests and the docker socket — 74c32b6

Checked instead of assumed, and you were right to flag it:

repository:                            public
/api/v1/settings/repository            forks_disabled: false
/user/sign_up                          200, registration form

So the trigger is reachable by anyone with an account. Gated to same-repository pull requests; scheduled and dispatched runs are unaffected.

if: >-
  github.event_name != 'pull_request' ||
  github.event.pull_request.head.repo.full_name == github.repository

build.yml gets the same treatment when #10 adds its trigger — that belongs in that pull request, not this one.

4. docker cp . "$id:/w"563ecf3

Real, and it reproduces exactly as you described. Against the pinned image with a destination that does exist:

docker cp $dir   $id:/tmp  ->  /tmp/w            # nested
docker cp $dir/. $id:/tmp  ->  /tmp/marker.txt   # contents

Now docker cp ./., which asks for the contents whether or not the destination exists.

5. forge.label() outside the guard — 8590294

Fixed. It warns and files without the label. issues() stays fatal, for the reason you gave.

6. unwatched.length reddening the weekly run — deferred, deliberately

Agreed, and not changed. The separation I would want is not a second exit code: an unwatched declaration is a finding, and this tool's whole argument is that findings belong in the tracker rather than in a run's exit status. So the right version files an issue for it like everything else, and then the weekly run only goes red when the checker is actually broken. That is a bigger change than this pull request should carry, and the current behaviour is wrong only in being coarse — it does not close anything or say anything false. Left as a follow-up.

7. The test that was one line away — c069a7c

Added, in both files: fold on two versions of one variant, and decide through the full sequence — file, partial bump, complete bump — asserting it never resolves on the strength of the upgraded site. Also that an unversioned tag folds only with the identical string, since shapeKey returns the tag itself there.

48 tests, deno lint and deno fmt --check clean, deno check tools/dep-check/main.ts clean, dry run exits 0 and still reports all seven.

8, 9. The docs reflow — 4533047

Reverted to base rather than split into its own commit. Your #9 is why: nothing checks formatting in CI, and if deno fmt output moves between toolchains then committing 380 reflowed lines here just guarantees they churn back. Formatting the docs is a decision worth taking on purpose, on a chosen Deno version, in a pull request about that. docs/dependencies.md and docs/security.md keep their real content.

10, 11. On the record

Both noted, nothing changed. Thank you for confirming the deno task check errors pre-date the branch — that was worth having from someone other than me.

And you drew the line in the right place on the host key. It rests on the two local checks, and it should; a reviewer cannot establish that from a diff, and I would not want one opening a connection to production to try.


Every reproduction you offered, I could reproduce from the description alone, so I did not need the scripts — but the offer is what made them quick to confirm. The docker cp one in particular I would have argued with if you had not given the exact rule.

Pushed six commits at `ab305fc`. Both blockers fixed, the security question answered with a check rather than a guess, and the three moderates that were fixable are fixed. One is deferred, with the reason; nothing is disputed. You were right about the shape of #1 in a way I want to name before the fix: both blockers are the same mistake I said this change existed to prevent, made one layer out from where I was looking. Acting on an answer that does not mean what it is taken to mean, and a pin with nothing watching it. --- ## 1. One key must not hold two versions — `c069a7c` Fixed by folding, not by re-keying. `dedupe()` is now `fold()`, and it folds on the issue identity rather than on `kind+name+version`, producing one `Dependency` that carries every declaration in `sites` and reports the **oldest** version among them. That makes the partial bump you described a non-event. The issue is about the site still on `v6`, so it stays open — and correct — until every site has moved: ``` after a partial bump (build.yml -> v7.0.1, deps.yml missed): issue is about v6; sites = ["v7.0.1 build.yml:30", "v6 deps.yml:45"] decide -> {"do":"skip","why":"already reported as v7.0.1"} and once deps.yml moves too: decide -> {"do":"resolve","issue":42} ``` Which does mean the title reads `v6 → v7.0.1` for as long as one site lags, while another already sits on `v7.0.1`. That is the intended reading: the issue is not "a newer version exists", it is "this repository is not on it yet", and it should not stop saying so while that is true. I took your second suggestion as well, because you were right that it is the most useful sentence available. When sites disagree the issue says which version is where — > - declared at **different versions**: `v7.0.1` in `build.yml:30`, `v6` in `deps.yml:45` — this issue is about the oldest, and stays open until every one of them has moved — and the run logs it, so a `--dry-run` surfaces it too. Nothing else here watches for that. Two definitions had drifted apart and are now one: `depKey` lives in `discover.ts` next to the fold that needs it and is re-exported from `issues.ts`. The marker key and the fold key being the same key is load-bearing; two definitions of it was how this got in. `Declaration` is what one line says, `Dependency` is what one issue is about. Your inline note on `shapeKey` was right too — I left it alone, the collision was `depKey`'s use of it. ## 2. `.forgejo/deno.sh` — `563ecf3` Removed the pin rather than teaching discovery to read shell scripts. The image comes out of the Dockerfile's build-stage `FROM`, which is already watched: ```sh IMAGE=$(sed -n 's/^FROM *\(denoland\/deno:[^ ]*\).*/\1/p' "$DOCKERFILE" | head -n 1) ``` One declaration instead of two, and it is one the checker reads. It also closes a gap I had only asserted: the script's own comment says the toolchain must be the same image the Dockerfile builds with, and now that is enforced rather than maintained by hand. Empty `IMAGE` exits 1 with a message. ``` .forgejo files carrying a sha256 pin: [] $ sh .forgejo/deno.sh test --allow-read tools/dep-check/ -> 48 passed, 0 failed ``` ## 3. Fork pull requests and the docker socket — `74c32b6` Checked instead of assumed, and you were right to flag it: ``` repository: public /api/v1/settings/repository forks_disabled: false /user/sign_up 200, registration form ``` So the trigger is reachable by anyone with an account. Gated to same-repository pull requests; scheduled and dispatched runs are unaffected. ```yaml if: >- github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ``` `build.yml` gets the same treatment when #10 adds its trigger — that belongs in that pull request, not this one. ## 4. `docker cp . "$id:/w"` — `563ecf3` Real, and it reproduces exactly as you described. Against the pinned image with a destination that does exist: ``` docker cp $dir $id:/tmp -> /tmp/w # nested docker cp $dir/. $id:/tmp -> /tmp/marker.txt # contents ``` Now `docker cp ./.`, which asks for the contents whether or not the destination exists. ## 5. `forge.label()` outside the guard — `8590294` Fixed. It warns and files without the label. `issues()` stays fatal, for the reason you gave. ## 6. `unwatched.length` reddening the weekly run — deferred, deliberately Agreed, and not changed. The separation I would want is not a second exit code: an unwatched declaration is a *finding*, and this tool's whole argument is that findings belong in the tracker rather than in a run's exit status. So the right version files an issue for it like everything else, and then the weekly run only goes red when the checker is actually broken. That is a bigger change than this pull request should carry, and the current behaviour is wrong only in being coarse — it does not close anything or say anything false. Left as a follow-up. ## 7. The test that was one line away — `c069a7c` Added, in both files: `fold` on two versions of one variant, and `decide` through the full sequence — file, partial bump, complete bump — asserting it never resolves on the strength of the upgraded site. Also that an unversioned tag folds only with the identical string, since `shapeKey` returns the tag itself there. 48 tests, `deno lint` and `deno fmt --check` clean, `deno check tools/dep-check/main.ts` clean, dry run exits 0 and still reports all seven. ## 8, 9. The docs reflow — `4533047` Reverted to base rather than split into its own commit. Your #9 is why: nothing checks formatting in CI, and if `deno fmt` output moves between toolchains then committing 380 reflowed lines here just guarantees they churn back. Formatting the docs is a decision worth taking on purpose, on a chosen Deno version, in a pull request about that. `docs/dependencies.md` and `docs/security.md` keep their real content. ## 10, 11. On the record Both noted, nothing changed. Thank you for confirming the `deno task check` errors pre-date the branch — that was worth having from someone other than me. And you drew the line in the right place on the host key. It rests on the two local checks, and it should; a reviewer cannot establish that from a diff, and I would not want one opening a connection to production to try. --- Every reproduction you offered, I could reproduce from the description alone, so I did not need the scripts — but the offer is what made them quick to confirm. The `docker cp` one in particular I would have argued with if you had not given the exact rule.
thisilike left a comment

Third pass, at ab305fc against base 1ae4844. Every inline comment is on a line this pull request adds.

Both blockers are properly dead, and I checked by running rather than reading: 48 tests pass, deno lint clean over 10 files, deno fmt --check clean over 14, dry run exits 0 with all seven dependencies. Dropping to a comment — nothing left here blocks a merge.

Two of your fixes are better than what I proposed, and it is worth saying which and why:

The fold. I suggested folding or reporting the disagreement; you did both, and folding at the oldest version is the part that makes it correct rather than merely different. The issue now stays true while the repository catches up, which is the property the whole closed-issue rule depends on. Reporting the newest would have been the obvious reading of "one issue per thing" and would have kept the bug.

The toolchain pin. I offered two options — teach discovery to read shell scripts, or move the pin somewhere watched — and you took a third that is better than either: delete the second declaration entirely and derive it from the FROM that already exists. Verified:

$ grep -rln "sha256:" .forgejo docker
docker/Dockerfile

$ sed -n 's/^FROM  *\(denoland\/deno:[^ ]*\).*/\1/p' docker/Dockerfile | head -n 1
denoland/deno:2.5.6@sha256:3ea71953…f166aa

One declaration, and it is one the checker reads. It also turns "CI must use the same toolchain the image is built with" from a comment into something enforced.

The fork check was the right way to answer #3forks_disabled: false and an open registration form is a fact, and I had only been able to offer a conditional. And I think you are right to defer #6: an unwatched declaration is a finding, findings belong in the tracker, and a second exit code would be the wrong shape of fix. Better reasoning than my original comment.

Everything below is minor. Two of them are one fix.


1 and 2 — split() and fold() disagree with version.ts about what a version is

version.ts states it outright, in the comment on compare: "these are RELEASES, v6 and v6.0.0 are the same one." split() decides with a string Set instead, and fold()'s tie-break inherits the same gap:

compare(v6, v6.0.0) = 0        (same release, by this tool's own rule)
fold([v6, v6.0.0]) -> v6       split() says they disagree: true
fold([v6.0.0, v6]) -> v6.0.0   ← input order decides

issue body: "- declared at **different versions**: `v6` in `a.yml:1`,
             `v6.0.0` in `b.yml:2` — this issue is about the oldest…"

So the new disagreement warning — the most valuable sentence in the tool — fires on a distinction the tool elsewhere denies, and when it does, which version lands in the title, in the marker's from, and in settled depends on directory read order.

No such pair exists in this repository today, so this is latent rather than live. One fix covers both: compare parsed nums and fall back to string equality only when parseVersion returns null — which is the same rule behind() already uses two functions up.

3 — Dependency.where is now dead

Inherited from Declaration, maintained by fold (at.where = d.where), and read by nothing: main.ts and issues.ts both go through sites. It is a field that looks authoritative and holds only the oldest site's line. Either drop it, or stop having Dependency extend Declaration so the type says what it means.

4 — deno.sh is now coupled to Dockerfile syntax, and that is worth knowing

FROM --platform=$BUILDPLATFORM denoland/deno:…, or a registry-qualified name, does not match the sed and the script exits 1 with a clear message. Failing loud is right and I am not asking for it to change. But a Dockerfile refactor now breaks CI from a file the Dockerfile never mentions, and the only thing that says so is a comment in the shell script. One line in docs/dependencies.md under the re-pinning procedure would put it where someone editing that FROM will see it.

5 — confirm the if: gate on this pull request's own run

If github.event.pull_request.head.repo.full_name does not populate on this Forgejo, the expression is false and the job skips on every pull request. That fails safe, but it silently removes the check you just added — and a skipped job does not look like a problem.

This pull request is same-repository, so its own run answers it: green means the gate works, skipped means it does not. Worth glancing at before merging rather than after.


Still open

The write path has not touched a real tracker. You have asked twice for a go-ahead on the scratch-repository rehearsal, and that is the right person to ask — it means creating and deleting a repository on the instance. Flagging it here so it does not get lost between rounds, not to press it.

Verdict

Approve after 1 and 2, which are one change. Everything else is optional and none of it blocks.

Two things I want on the record because they were yours: catching that container: denoland/deno cannot run a Node action, and that a bind mount from inside the runner's own container mounts nothing, are both findings that only exist because you ran the thing. My #6 asked for a rehearsal and got something better — a workflow that failed twice in public and is now known to work.

Third pass, at `ab305fc` against base `1ae4844`. Every inline comment is on a line this pull request adds. Both blockers are properly dead, and I checked by running rather than reading: **48 tests pass, `deno lint` clean over 10 files, `deno fmt --check` clean over 14, dry run exits 0 with all seven dependencies.** Dropping to a comment — nothing left here blocks a merge. Two of your fixes are better than what I proposed, and it is worth saying which and why: **The fold.** I suggested folding *or* reporting the disagreement; you did both, and folding at the **oldest** version is the part that makes it correct rather than merely different. The issue now stays true while the repository catches up, which is the property the whole closed-issue rule depends on. Reporting the newest would have been the obvious reading of "one issue per thing" and would have kept the bug. **The toolchain pin.** I offered two options — teach discovery to read shell scripts, or move the pin somewhere watched — and you took a third that is better than either: delete the second declaration entirely and derive it from the `FROM` that already exists. Verified: ``` $ grep -rln "sha256:" .forgejo docker docker/Dockerfile $ sed -n 's/^FROM *\(denoland\/deno:[^ ]*\).*/\1/p' docker/Dockerfile | head -n 1 denoland/deno:2.5.6@sha256:3ea71953…f166aa ``` One declaration, and it is one the checker reads. It also turns "CI must use the same toolchain the image is built with" from a comment into something enforced. The fork check was the right way to answer #3 — `forks_disabled: false` and an open registration form is a fact, and I had only been able to offer a conditional. And I think you are right to defer #6: an unwatched declaration is a finding, findings belong in the tracker, and a second exit code would be the wrong shape of fix. Better reasoning than my original comment. Everything below is minor. Two of them are one fix. --- ## 1 and 2 — `split()` and `fold()` disagree with `version.ts` about what a version is `version.ts` states it outright, in the comment on `compare`: *"these are RELEASES, `v6` and `v6.0.0` are the same one."* `split()` decides with a string `Set` instead, and `fold()`'s tie-break inherits the same gap: ``` compare(v6, v6.0.0) = 0 (same release, by this tool's own rule) fold([v6, v6.0.0]) -> v6 split() says they disagree: true fold([v6.0.0, v6]) -> v6.0.0 ← input order decides issue body: "- declared at **different versions**: `v6` in `a.yml:1`, `v6.0.0` in `b.yml:2` — this issue is about the oldest…" ``` So the new disagreement warning — the most valuable sentence in the tool — fires on a distinction the tool elsewhere denies, and when it does, which version lands in the title, in the marker's `from`, and in `settled` depends on directory read order. No such pair exists in this repository today, so this is latent rather than live. One fix covers both: compare parsed `nums` and fall back to string equality only when `parseVersion` returns null — which is the same rule `behind()` already uses two functions up. ## 3 — `Dependency.where` is now dead Inherited from `Declaration`, maintained by `fold` (`at.where = d.where`), and read by nothing: `main.ts` and `issues.ts` both go through `sites`. It is a field that looks authoritative and holds only the oldest site's line. Either drop it, or stop having `Dependency` extend `Declaration` so the type says what it means. ## 4 — `deno.sh` is now coupled to Dockerfile syntax, and that is worth knowing `FROM --platform=$BUILDPLATFORM denoland/deno:…`, or a registry-qualified name, does not match the `sed` and the script exits 1 with a clear message. Failing loud is right and I am not asking for it to change. But a Dockerfile refactor now breaks CI from a file the Dockerfile never mentions, and the only thing that says so is a comment in the shell script. One line in `docs/dependencies.md` under the re-pinning procedure would put it where someone editing that `FROM` will see it. ## 5 — confirm the `if:` gate on this pull request's own run If `github.event.pull_request.head.repo.full_name` does not populate on this Forgejo, the expression is false and the job skips on **every** pull request. That fails safe, but it silently removes the check you just added — and a skipped job does not look like a problem. This pull request is same-repository, so its own run answers it: green means the gate works, skipped means it does not. Worth glancing at before merging rather than after. --- ## Still open The write path has not touched a real tracker. You have asked twice for a go-ahead on the scratch-repository rehearsal, and that is the right person to ask — it means creating and deleting a repository on the instance. Flagging it here so it does not get lost between rounds, not to press it. ## Verdict Approve after 1 and 2, which are one change. Everything else is optional and none of it blocks. Two things I want on the record because they were yours: catching that `container: denoland/deno` cannot run a Node action, and that a bind mount from inside the runner's own container mounts nothing, are both findings that only exist because you ran the thing. My #6 asked for a rehearsal and got something better — a workflow that failed twice in public and is now known to work.
@ -0,0 +25,4 @@
# file, so a digest written here would go stale in silence. The build stage's
# FROM is the one declaration; it is already watched, and reading it from
# there is also what guarantees CI runs the toolchain the image is built with.
DOCKERFILE=docker/Dockerfile
Owner

Better than either option I offered. Deleting the second declaration beats teaching discovery to read shell scripts and beats moving the pin somewhere watched, because it leaves one declaration rather than two that have to agree. Verified:

$ grep -rln "sha256:" .forgejo docker
docker/Dockerfile

One note worth putting somewhere a reader will hit it: this couples CI to the Dockerfile's FROM syntax. FROM --platform=$BUILDPLATFORM denoland/deno:…, or a registry-qualified name, does not match and the script exits 1 — failing loud, which is right. But a Dockerfile refactor then breaks CI from a file the Dockerfile never mentions, and only this comment says so. A line under the re-pinning procedure in docs/dependencies.md would put it in front of whoever edits that FROM.

Better than either option I offered. Deleting the second declaration beats teaching discovery to read shell scripts *and* beats moving the pin somewhere watched, because it leaves one declaration rather than two that have to agree. Verified: ``` $ grep -rln "sha256:" .forgejo docker docker/Dockerfile ``` One note worth putting somewhere a reader will hit it: this couples CI to the Dockerfile's `FROM` syntax. `FROM --platform=$BUILDPLATFORM denoland/deno:…`, or a registry-qualified name, does not match and the script exits 1 — failing loud, which is right. But a Dockerfile refactor then breaks CI from a file the Dockerfile never mentions, and only this comment says so. A line under the re-pinning procedure in `docs/dependencies.md` would put it in front of whoever edits that `FROM`.
@ -0,0 +37,4 @@
# forks enabled and open registration, so the trigger is reachable by
# anyone; secrets are withheld from fork runs, but the runner that holds
# them on the next push to main is not.
if: >-
Owner

Right answer, and checking forks_disabled and the registration form rather than reasoning about it is what settled it — I could only offer a conditional.

One thing to confirm rather than assume, in the other direction: if github.event.pull_request.head.repo.full_name does not populate on this Forgejo, this expression is false and the job skips on every pull request. That fails safe, but it silently removes the check you just added, and a skipped job does not look like a problem.

This pull request is same-repository, so its own run answers it — green means the gate works, skipped means it does not. Worth a glance before merging rather than after.

Right answer, and checking `forks_disabled` and the registration form rather than reasoning about it is what settled it — I could only offer a conditional. One thing to confirm rather than assume, in the other direction: if `github.event.pull_request.head.repo.full_name` does not populate on this Forgejo, this expression is false and the job skips on **every** pull request. That fails safe, but it silently removes the check you just added, and a skipped job does not look like a problem. This pull request is same-repository, so its own run answers it — green means the gate works, skipped means it does not. Worth a glance before merging rather than after.
@ -0,0 +41,4 @@
* asserting it is current. Reporting the oldest keeps the issue open until
* every site has moved, which is when it is actually true.
*/
export interface Dependency extends Declaration {
Owner

where is inherited here, maintained by fold (at.where = d.where), and read by nothing — main.ts and issues.ts both go through sites.

A field that looks authoritative and actually holds only the oldest site's line. Either drop it, or stop extending Declaration so the type says what a Dependency now is: a set of sites plus the oldest version among them.

The Declaration / Dependency split itself is a real improvement — the old type was doing both jobs and that is part of how the two-versions bug hid.

`where` is inherited here, maintained by `fold` (`at.where = d.where`), and read by nothing — `main.ts` and `issues.ts` both go through `sites`. A field that looks authoritative and actually holds only the oldest site's line. Either drop it, or stop extending `Declaration` so the type says what a `Dependency` now is: a set of sites plus the oldest version among them. The `Declaration` / `Dependency` split itself is a real improvement — the old type was doing both jobs and that is part of how the two-versions bug hid.
@ -0,0 +240,4 @@
`${dep.kind}:${dep.name}:${shapeKey(dep.version)}`;
/** strictly behind, among tags of one shape */
function behind(a: string, b: string): boolean {
Owner

behind is strictly-less-than, which is right — but it means fold's tie-break is decided by input order whenever compare returns 0:

fold([v6, v6.0.0]) -> v6
fold([v6.0.0, v6]) -> v6.0.0

Which version lands in the issue title, in the marker's from, and in settled then depends on the order Deno.readDir happens to yield. Latent — no such pair exists in this repository today — but it is nondeterminism sitting next to the identity.

Same fix as split(): once equal-by-nums counts as equal, there is no tie to break arbitrarily, because the two are genuinely the same release.

`behind` is strictly-less-than, which is right — but it means `fold`'s tie-break is decided by input order whenever `compare` returns 0: ``` fold([v6, v6.0.0]) -> v6 fold([v6.0.0, v6]) -> v6.0.0 ``` Which version lands in the issue title, in the marker's `from`, and in `settled` then depends on the order `Deno.readDir` happens to yield. Latent — no such pair exists in this repository today — but it is nondeterminism sitting next to the identity. Same fix as `split()`: once equal-by-`nums` counts as equal, there is no tie to break arbitrarily, because the two are genuinely the same release.
@ -0,0 +258,4 @@
* `Dependency`. One key holding two versions is what let a run close an issue
* because one of two sites had been upgraded.
*/
export function fold(decls: Declaration[]): Dependency[] {
Owner

This is the fix, and folding at the oldest is the part that makes it right rather than merely different.

I suggested folding or reporting the disagreement and you did both, but the oldest-version choice is yours and it is what makes the issue stay true while the repository catches up — which is what the closed-issue rule depends on. Folding at the newest would have been the obvious reading of "one issue per thing" and would have preserved the bug in a form that was harder to see.

Verified the sequence: partial bump keeps the issue open, complete bump resolves it.

This is the fix, and folding at the **oldest** is the part that makes it right rather than merely different. I suggested folding *or* reporting the disagreement and you did both, but the oldest-version choice is yours and it is what makes the issue stay true while the repository catches up — which is what the closed-issue rule depends on. Folding at the newest would have been the obvious reading of "one issue per thing" and would have preserved the bug in a form that was harder to see. Verified the sequence: partial bump keeps the issue open, complete bump resolves it.
@ -0,0 +276,4 @@
}
/** the versions this dependency is declared at, when its sites disagree */
export function split(dep: Dependency): Declaration[] | null {
Owner

Minor, and the same fix as the one on behind below. This decides with a string Set, but version.ts says on compare: "these are RELEASES, v6 and v6.0.0 are the same one."

compare(v6, v6.0.0) = 0        (same release, by this tool's own rule)
split() says they disagree: true

issue body: "- declared at **different versions**: `v6` in `a.yml:1`,
             `v6.0.0` in `b.yml:2` — this issue is about the oldest…"

So the disagreement warning — which is the most valuable sentence this tool produces, and the reason I suggested it — fires on a distinction the tool elsewhere denies.

Compare parsed nums, falling back to string equality only when parseVersion returns null. That is exactly what behind() already does two functions up; the rule just did not make it into here.

**Minor, and the same fix as the one on `behind` below.** This decides with a string `Set`, but `version.ts` says on `compare`: *"these are RELEASES, `v6` and `v6.0.0` are the same one."* ``` compare(v6, v6.0.0) = 0 (same release, by this tool's own rule) split() says they disagree: true issue body: "- declared at **different versions**: `v6` in `a.yml:1`, `v6.0.0` in `b.yml:2` — this issue is about the oldest…" ``` So the disagreement warning — which is the most valuable sentence this tool produces, and the reason I suggested it — fires on a distinction the tool elsewhere denies. Compare parsed `nums`, falling back to string equality only when `parseVersion` returns null. That is exactly what `behind()` already does two functions up; the rule just did not make it into here.
@ -0,0 +88,4 @@
// one of these issues and bumping only one of the places. Said out loud
// here as well as in the issue, because a dry run is where it gets read.
for (const dep of deps) {
const sites = split(dep);
Owner

Putting the disagreement in the run log as well as the issue is the right call — a dry run is where this actually gets read, and it is the one thing here nothing else in the repository watches for.

It is also the code that surfaces the split() bug: until that compares numerically, this warns about v6 versus v6.0.0, which compare says is the same release.

Putting the disagreement in the run log as well as the issue is the right call — a dry run is where this actually gets read, and it is the one thing here nothing else in the repository watches for. It is also the code that surfaces the `split()` bug: until that compares numerically, this warns about `v6` versus `v6.0.0`, which `compare` says is the same release.
@ -0,0 +139,4 @@
// read labels is an ordinary configuration, and aborting on it would
// lose the whole run to a cosmetic field.
console.warn(
` ! could not read labels (${(e as Error).message}); filing without`,
Owner

This is the one I most expected to be argued with rather than fixed, since the failure is hypothetical. Warning and filing without the label is exactly the rule the rest of the loop follows, and the doc comment on Forge.label now describes what the code does instead of what it intended.

This is the one I most expected to be argued with rather than fixed, since the failure is hypothetical. Warning and filing without the label is exactly the rule the rest of the loop follows, and the doc comment on `Forge.label` now describes what the code does instead of what it intended.
thisilike approved these changes 2026-08-10 15:26:12 +02:00
thisilike left a comment

Approving at ab305fc.

Bookkeeping first, since it is my fault you were looking for this: the third pass went up as a COMMENT review (#issuecomment-247), and Forgejo renders those as loose comments with no verdict — so it neither showed as a review nor cleared the re-request you filed at 14:26. The detail is all in that one; this is the verdict that was missing from it.

Approved on the substance, not on fatigue. Both blockers from the second pass are dead, verified by running rather than reading: 48 tests pass, deno lint clean over 10 files, deno fmt --check clean over 14, dry run exits 0 with all seven dependencies, and grep -rln "sha256:" .forgejo docker now returns docker/Dockerfile alone.

Two of the fixes are better than what I asked for:

  • the fold at the oldest version — folding was my suggestion, oldest was yours, and it is the half that makes the issue stay true while the repository catches up. Folding at the newest would have looked equally reasonable and kept the bug;
  • deleting the second toolchain pin rather than teaching discovery to read shell scripts. One declaration instead of two that have to agree, and it makes "CI runs the toolchain the image is built with" enforced instead of remembered.

Answering #3 with forks_disabled and the registration form, instead of arguing about it, is what settled that one. And your reasoning for deferring #6 is better than the comment it answers — an unwatched declaration is a finding, findings belong in the tracker, and a second exit code is the wrong shape.

One thing to do, not blocking. split() compares versions as strings while version.ts says on compare that "v6 and v6.0.0 are the same one", so the disagreement warning fires on a distinction the tool elsewhere denies — and fold's tie-break then falls to readDir order. One change fixes both: compare parsed nums, fall back to strings only when parseVersion returns null, exactly as behind() already does. Latent today, since no such pair exists in this repository. Follow-up is fine.

Two to glance at rather than fix. Confirm this pull request's own run went green rather than skipped — if github.event.pull_request.head.repo.full_name does not populate here, the new if: gate fails safe by skipping every pull request, which silently removes the check. And Dependency.where is now dead: maintained by fold, read by nothing.

Still yours to call: the write path has not touched a real tracker. You have asked twice about the scratch-repository rehearsal; that decision is with @julian, not with the review.

Good change. The pins were right from the first commit and the checker took three rounds to become worth trusting — which is the correct order for those two things to have happened in.

Approving at `ab305fc`. Bookkeeping first, since it is my fault you were looking for this: the third pass went up as a `COMMENT` review ([#issuecomment-247](https://git.imhof.cloud/OpsDeck/core/pulls/11#issuecomment-247)), and Forgejo renders those as loose comments with no verdict — so it neither showed as a review nor cleared the re-request you filed at 14:26. The detail is all in that one; this is the verdict that was missing from it. **Approved on the substance, not on fatigue.** Both blockers from the second pass are dead, verified by running rather than reading: 48 tests pass, `deno lint` clean over 10 files, `deno fmt --check` clean over 14, dry run exits 0 with all seven dependencies, and `grep -rln "sha256:" .forgejo docker` now returns `docker/Dockerfile` alone. Two of the fixes are better than what I asked for: - **the fold at the *oldest* version** — folding was my suggestion, oldest was yours, and it is the half that makes the issue stay true while the repository catches up. Folding at the newest would have looked equally reasonable and kept the bug; - **deleting the second toolchain pin** rather than teaching discovery to read shell scripts. One declaration instead of two that have to agree, and it makes "CI runs the toolchain the image is built with" enforced instead of remembered. Answering #3 with `forks_disabled` and the registration form, instead of arguing about it, is what settled that one. And your reasoning for deferring #6 is better than the comment it answers — an unwatched declaration is a finding, findings belong in the tracker, and a second exit code is the wrong shape. **One thing to do, not blocking.** `split()` compares versions as strings while `version.ts` says on `compare` that *"`v6` and `v6.0.0` are the same one"*, so the disagreement warning fires on a distinction the tool elsewhere denies — and `fold`'s tie-break then falls to `readDir` order. One change fixes both: compare parsed `nums`, fall back to strings only when `parseVersion` returns null, exactly as `behind()` already does. Latent today, since no such pair exists in this repository. Follow-up is fine. **Two to glance at rather than fix.** Confirm this pull request's own run went green rather than skipped — if `github.event.pull_request.head.repo.full_name` does not populate here, the new `if:` gate fails safe by skipping every pull request, which silently removes the check. And `Dependency.where` is now dead: maintained by `fold`, read by nothing. **Still yours to call:** the write path has not touched a real tracker. You have asked twice about the scratch-repository rehearsal; that decision is with @julian, not with the review. Good change. The pins were right from the first commit and the checker took three rounds to become worth trusting — which is the correct order for those two things to have happened in.
version.ts states it on compare: these are RELEASES, v6 and v6.0.0 are the
same one. split() decided with a string Set instead, so the disagreement
warning fired on a distinction the rest of the tool denies.

The tie-break was the half with teeth. behind() returns false for equal
releases, so with v6 and v6.0.0 declared in two places neither was older and
the winner was whichever Deno.readDir returned first. That version is the
issue title, the marker's from that decide() compares against, and the tag the
settled check looks for in the registry's list — so all three flipped between
runs and the issue was rewritten every Monday having not changed, which is the
one property the design rests on: a run that finds nothing new writes nothing
at all.

Both now use the rule behind() already used — compare parsed nums, fall back
to string equality only when parseVersion returns null — plus an explicit
tie-break on the smaller string, so the answer does not depend on read order.
Latent: no such pair exists in this repository today, and the test asserts
both orders give the same answer.

Dependency no longer extends Declaration. It inherited a where that fold kept
pointing at the oldest site and nothing ever read — issues.ts and main.ts both
go through sites. The fields the two share are now a Named, which is also
exactly what depKey takes.

Verified in the pinned image: 49 passed, lint and fmt clean, and the dry run
is unchanged at 7 dependencies.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs(dependencies): what a dry run does not reach, and the FROM deno.sh reads
All checks were successful
Dependency Check / dependencies (pull_request) Successful in 1m11s
ab50124401
Two things this document did not say, in the one whose job is to be accurate
about what is watched and what is not.

--dry-run was described as doing every read. It does not open the tracker at
all: main() leaves forge null, so configFromEnv, label() and issues() never
run and every decision is taken against an empty issue list — which is why a
dry run reports every dependency as one it would file. The forge half is the
part a dry run cannot exercise, and that is worth stating rather than leaving
"the whole read path" to imply otherwise.

And the re-pinning procedure did not mention that .forgejo/deno.sh finds the
toolchain by matching ^FROM  *denoland/deno: in the Dockerfile. Rewriting that
line with --platform= or a registry-qualified name breaks every deps.yml step
from a file the Dockerfile never mentions. It fails loud, which is the right
failure; the note belongs where someone editing the FROM will see it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
julian merged commit 235ce77a26 into main 2026-08-10 15:46:07 +02:00
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!11
No description provided.