fix(dep-check): an image built here is not a dependency a registry answers for #41
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/issue-25-dep-check-local-image"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #25.
The noise
docker/dev-rig/Dockerfile:10isFROM opsdeck:rig-base, and no registry has that image —docker/dev-rig/up.sh:57builds it out ofdocker/Dockerfileand tags it locally. Discovery walks the tree rather than reading a hardcoded list of paths, deliberately, so it finds the line,imageDependencysplits it like any other reference, and the unqualified name resolves against Docker Hub wherelibrary/opsdeckdoes not exist and anonymous auth answers 401. The tag carries no version either, so a lookup that succeeded would still have nothing to compare.It never made the run red — the exit rule is
failed || unwatched.length || unreachable === reports.length— so this was never breakage. What it cost is the property the tool is built around: a run that finds nothing writes nothing, so anything it does print is worth reading. A line that prints forever and can never be resolved is a line that gets skipped, and a genuine registry outage on a real dependency prints the same shape.Which option
The issue lists four and prescribes none. This is option 1, the marker.
unwatched— reads correctly, butunwatched.lengthtakes the run red, so it trades permanent noise for permanent failure unless the bucket is first split into "cannot be watched, deliberately" and "should be watched, isn't". Bigger change to the exit-code rule than this line is worth.$means operator indirection) and makes the base implicit to whoever reads the Dockerfile.There is also a fifth the issue does not list: never ask a registry about a tag
parseVersioncannot read — the 401 disappears with no marker at all, anddebian:bookworm-slimstops paying a wasted round trip every run. It is not a drop-in:settled(main.tsinsurvey) needs the complete tag list to close an issue whose tag was later changed to an unversioned one (issues.ts:118-127, andresolvedCommentsays so in as many words), so the lookup cannot be skipped unconditionally. Named here so it does not read as overlooked.Option 1 is closest to the existing convention: discovery already reads a trailing
# v7.0.1comment to keep a SHA-pinned action answerable.The marker
Read above a
FROMor aCOPY --from=; that line then stops being a declaration at all — not reported, not counted, not red.Its own line, never trailing. Docker has no inline comments —
#inside an instruction is an argument, soFROM x # dep-check: localfails the build withFROM requires either one or three arguments. The parser deliberately does not read the marker in that position either, so the form it documents is the form that builds. (Confirmed word-for-word by review, on a machine that has docker.)It reaches exactly one instruction — at any number of comment lines' distance. A blank line or any other instruction ends it; further comment lines do not, which is the shape the real file has (marker, then the sentence explaining it, then the
FROM). That reach is wider than "the line below", and a skip used to leave no trace anywhere — the drift the review flagged: the dayopsdeck:rig-basebecomes a published image, whoever edits theFROMalso has to spot a paragraph several comment lines up, and missing it would unwatch a real dependency silently. So discovery now carries every ref the marker silences out in a third bucket,skipped, and a dry run prints each one:Not counted, never red — and not by-hand-only. An earlier revision of this description (and two sentences in the diff) claimed the line was "silent in CI"; that was false, and review caught it.
deps.ymlruns--dry-runon every same-repository pull request, so the pull request that edits a markedFROMis exactly the run whose CI log shows this line move or go missing — including the drift case above, and a marker quietly un-marked by anARGslipped above a parameterisedFROM, the likeliest real edit to defeat it. The weekly and on-demand runs both take the write path, without--dry-run, so neither ever prints it — said that way around deliberately, with no claim about which runs are silent, because a fork's pull request skips the job entirely and prints nothing too. (A previous revision claimed the weekly run was "the one that stays silent"; review caught the exclusivity.) The comment inmain.tsand the paragraph indocs/dependencies.mdnow say exactly that.A mistyped marker leaves the line watched. The safe direction: a typo costs a noisy line, the other direction silences a real dependency. The inverse edge is documented too: a mid-sentence mention of the marker in a comment is not read, but a comment that begins with the marker text is a marker — the
LOCAL_MARKERdocstring now says a Dockerfile comment documenting the convention must not open with it.Marking a line local orphans any issue already open about it.
resolveonly fires for dependencies still being discovered, and a marked line is no longer discovered — so an open issue about it would sit asserting an upgrade nobody watches for. No live impact here (rig-base always came back witherrorset, so no issue was ever filed), but the docs now say: close such an issue by hand, the checker never will.Stage names still register. The marker is read below the
ASname, so a markedFROM x AS buildstill declaresbuildand a laterCOPY --from=buildis not mistaken for an image.Dockerfiles only. Neither workflow reader looks for it:
workflowImagesFromdoes not, so acontainer: image:cannot be marked local, andactionsFromdoes not, so neither can auses:line. Nothing needs it today; the docstring now names both readers, and a test holds thecontainer:half true.After
Exit 0, nothing unreachable. (The
+onsetup-buildx-actionis real upstream movement since the first run of this branch, not this diff — review saw the same.)Tests
Seven marker tests in
tools/dep-check/discover_test.ts, withskippedassertions threaded through: the markedFROMandCOPY --from=land inskippedwith their file:line and nowhere counted; a marked line still names its stage; a marked untaggedFROMis skipped, not reported; the marker reaches exactly one instruction (blank line, other instruction, and a following comment line that must not end it) with theARG-between case additionally assertingskippedstays empty — the watched direction; a mistyped marker leaves the line watched; the inline form is not read; andworkflowImagesFromnever reads the marker at all.Of the original six, four fail with the source change stashed — the review measured four where this description previously claimed three. The mistyped-marker and inline tests pass either way; they are guards against a future loosening of
LOCAL_MARKER, not detectors of this change, and are kept for that. Theskippedassertions do not run against the old source at all — there is noskippedfield to read.Gates, re-run after the fourth (comment- and doc-only) commit:
deno task check,deno lint tools/dep-check/,deno fmt --check,deno test --allow-read tools/dep-check/(62 passed), and the--dry-runabove against the real tree.docs/dependencies.mdgains the marker under "What it will not tell you", including the dry-run skip line, what its movement or absence means, and which runs print it — plus a bullet for# syntax=docker/dockerfile:1, an image reference on a mutable tag living on a comment line that discovery never reads, and the orphaned-issue clause above. The sample fence now carries the output's real two-space indent; thechecking 7 dependenciesline sits inside the fence becausedeno fmtstrips a fence's common leading indent, so the flush line is what anchors the relative one.`docker/dev-rig/Dockerfile` is `FROM opsdeck:rig-base`, and that image comes from no registry — `docker/dev-rig/up.sh` builds it out of `docker/Dockerfile` and tags it locally. Discovery walks the tree rather than reading a list of paths, deliberately, so it finds the line, resolves the unqualified name against Docker Hub, and prints ? opsdeck rig-base: registry answered HTTP 401 on every run. It never made the run red, and it can never be resolved either. What it cost is the property the tool is built around: a run that finds nothing writes nothing, so anything it does print is worth reading. A line that prints forever is a line that gets skipped, and a real registry outage prints the same shape. Nothing in `FROM opsdeck:rig-base` distinguishes a locally built image from a registry one, so the line has to say so itself: # dep-check: local — up.sh builds this, no registry has it FROM opsdeck:rig-base The marker is read above a `FROM` or a `COPY --from=`, and that line then stops being a declaration at all — not reported, not counted toward `unwatched`, which would have traded permanent noise for permanent red. Its own line, never trailing: docker has no inline comments, so `FROM x # dep-check: local` would fail the build with "FROM requires either one or three arguments". It reaches exactly one instruction — a blank line or any other instruction ends it, further comment lines do not — because an entry that outlives what it was written for is the ignore-list failure this form exists to avoid. A mistyped marker leaves the line watched, which is the safe direction. `--dry-run` now reports 7 dependencies with nothing unreachable. Closes #25 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Re-reviewed the whole diff, re-ran every gate rather than taking the description's word for them, and reproduced the one claim it flags as unverified. The parser is correct — I tried to break it and could not. Two things to fix before merge, both small, and one of them is a sentence this PR adds that is not true.
Verified independently, not taken on trust
deno test --allow-read tools/dep-check/→ 61 passed.deno lint tools/dep-check/,deno fmt --check,deno check tools/dep-check/→ clean.opsdeck rig-baseis the only thing lost —actions/checkout v7.0.1(3 sites),docker/login-action,docker/setup-buildx-action,docker/build-push-action,denoland/deno 2.9.5,debian bookworm-slim,denoland/deno bin-2.9.5are all still found at the same lines.FROM debian:bookworm-slim # dep-check: local: Word for word what the docstring says. That "worth a second pair of eyes" is settled — the form documented is the form that builds.discover.tsreverted toHEAD~1, four of the six new tests fail, not three.FROM --platform=…(marker still reaches it), CRLF, tabs inside the marker,FROM scratchconsuming it, marker at EOF, a heredoc body (RUN <<EOF/# dep-check: local/EOF— theEOFline consumes it, so the nextFROMstays watched), aRUN … \continuation the same way,# dep-check: locally(the\bholds), a prose comment mentioning the marker mid-sentence (# we dropped the dep-check: local marker here— not read, because the regex is effectively anchored to the#), and a markedFROMfollowed by a realCOPY --from=denoland/deno:bin-2.9.5, which is still watched.FROMlines:.forgejo/deno.sh:31andtools/ci_pins_test.ts:38both anchor on^FROM *denoland/deno:indocker/Dockerfile, so a comment above dev-rig'sFROMreaches neither.1. "it cannot outlive the line it was written for" is not true (blocking — one sentence, one line of output)
tools/dep-check/discover.ts:181anddocs/dependencies.md:134both make that claim, and the whole argument for this form over option 2 rests on it. But comment lines do not end the marker's reach — deliberate, with a test enshrining it — so the real bound is "one instruction, at any number of comment lines' distance". Ran it:→
deps: [].The real Dockerfile is the shape that makes this reachable: the marker is a two-line paragraph at the tail of a nine-line block about something else entirely. The day
opsdeck:rig-basebecomes a published image — dev-rig pulling a real base rather than building one — whoever edits line 12 also has to notice and delete a paragraph six lines up that reads as part of the isolation comment. Miss it, and a versioned, published dependency is unwatched forever, silently. That is the exact failure this tool exists to catch, and right now the marker is the only thing in the repository that can cause it.The reach rule should stay as it is — the two-line marker paragraph in the real file needs it. What is missing is that the skip leaves no trace anywhere:
checking 7 dependenciesand nothing else. There is no output, under any flag, that says a line was skipped, so this drift can never be noticed from a run.Cheapest fix that keeps the property the whole design rests on ("a run that finds nothing writes nothing, so anything it prints is worth reading"): carry the skipped refs out of
dockerfileImagesFromin a third bucket and print them only under--dry-run, where a human is already reading:Not counted, not red, invisible in CI. It answers "did my marker land where I meant it", and it makes §2 self-correcting. Plus: reword the two sentences to say "one instruction" rather than "the line it was written for".
2. An
ARGunder the marker silently un-marks it (low — same fix)→
deps: ["opsdeck:rig-base"]. The 401 is back. Correct per the documented rule and it fails in the safe direction, butARG-before-FROMis the idiom for a parameterised base, so this is the likeliest way the marker quietly stops working in practice.discover_test.ts:221already constructs this exact case; it just is not surfaced anywhere a person would see it. The--dry-runline above is what makes it noticeable.3. Two of the six tests do not detect this change (nit)
discover_test.ts:234(mistyped) and:248(inline) pass identically againstHEAD~1. Both are real guards against a future loosening — ifLOCAL_MARKERwere ever tested against non-comment lines, the inline one catches it — so keep them. The description should just not imply all six are change-detecting.4.
workflowImagesFromhas no marker (nit)container: image: opsdeck:rig-basein a workflow would produce the same permanent 401 with no way out, and a#line above it is not read. Nothing needs it today. One clause in theLOCAL_MARKERdocstring saying "Dockerfiles only" stops the next reader assuming otherwise.On the option list — no action, but name it
There is a fifth option the description does not consider: do not ask a registry about a tag
parseVersioncannot read.opsdeck:rig-baseis unversioned,pickLatestreturns null, and the answer is discarded — the 401 is paid for nothing, anddebian:bookworm-slimpays the same wasted round trip on every run.It is not a drop-in replacement, and I checked that before raising it:
settled(main.ts:64) needs the tag list to close an issue whose tag was later changed to an unversioned one (issues.ts:118-127, andresolvedCommentsays so in as many words), so the lookup cannot be skipped unconditionally. Worth one line in the description so the next reader does not re-derive that and conclude it was overlooked.Everything else holds: the marker read below the
ASname,unwatchedcorrectly not inheriting the skip, the exit rule unaffected (unreachable === reports.lengthbehaved the same before and after, since rig-base was always one of the unreachable),deno fmtfence style matching the rest of the doc, one clean conventional-commit, and no drift on main in any touched path.@ -116,0 +131,4 @@`FROM x # dep-check: local` fails the build with`FROM requires either one or three arguments`. It reaches exactly oneinstruction — a blank line or any other instruction ends it — because an entrythat outlives what it was written for is the ignore-list failure this formSame overclaim as
discover.ts:181: "outlives what it was written for" is only true at instruction granularity. The paragraph already says "a blank line or any other instruction ends it" one clause earlier, which is correct — it is this clause that promises more than the code delivers. Say "one instruction" and drop the "cannot outlive" framing, or state plainly that comment lines do not end it (which the reader needs anyway, since the shipped marker relies on exactly that).@ -167,0 +178,4 @@** A comment directly above the line rather than an ignore list somewhere* else, for the same reason an action's tag lives next to its SHA: the next* reader of the Dockerfile sees it, and it cannot outlive the line it wasThis is the sentence the whole form rests on, and it is not accurate. A comment line does not end the marker's reach (line 219 below, and there is a test asserting it), so the bound is "one instruction", not "the line it was written for" — and any number of comment lines can sit between.
Reproduced: marker, four unrelated comment lines, then
FROM debian:bookworm-slim→deps: []. In the realdocker/dev-rig/Dockerfilethe marker is a two-line paragraph at the tail of a nine-line block about the nested engine, so the day that base becomes a published image, the person editing theFROMhas to also spot a paragraph six lines up. Miss it and a real dependency is unwatched forever, with nothing in the output saying so.Reword to "one instruction", and see the review body for the
--dry-runline that makes the drift visible.@ -184,0 +216,4 @@marked = false;return;}if (/^\s*#/.test(line)) {The blank-line and instruction resets are what the comment above claims, but this branch is the third case and it is the one that is unbounded: comments neither set nor clear the marker unless they are the marker, so the reach is arbitrarily long across a comment block.
That is the right behaviour for the real file's two-line marker paragraph — the problem is only that a skip is invisible afterwards. Suggest returning the skipped refs in a third bucket on
Scansomain.tscan print them under--dry-run:Not counted, not red, silent in CI.
@ -139,0 +218,4 @@["debian"],);// and an instruction that is not a FROM consumes it tooThis case is worth more than a line in a combined test.
ARG-before-FROMis the standard idiom for a parameterised base, so this is the likeliest way the marker stops working in a real edit:→
deps: ["opsdeck:rig-base"], the 401 back with no indication why the marker stopped applying. Fails in the safe direction, agreed — it is the diagnosability that is missing, which the--dry-runskip line in the review body fixes.All four points addressed in
0ae2301, plus the description rewritten where it overclaimed. Point by point:1. The invisible skip (blocking). Taken exactly as suggested:
dockerfileImagesFromcarries every ref the marker silences out in a third bucket,skipped(ref + file:line), merged throughdiscover(), andmain.tsprints each one only under--dry-run:Not counted in
checking N dependencies, no effect on the exit rule, nothing printed in CI mode. Verified against the real tree — the line above is the actual output, exit 0. Both overclaiming sentences are gone: theLOCAL_MARKERdocstring anddocs/dependencies.mdnow state the bound as one instruction at any number of comment lines' distance, say explicitly that comment lines do not end the reach, and point at the dry-run line as the place drift becomes visible.2.
ARGun-marking. Covered by the same dry-run line, as you said — the skip line goes missing and the 401 returns, which is now a visible diff between two dry runs instead of nothing. The test for that case (discover_test.ts, the reach test) now also assertsskippedstays empty, with a comment naming it the likeliest real edit to defeat the marker.3. Test claims. The description now says four of the original six fail with the source change stashed (your measurement was right, mine was wrong), and names the mistyped and inline tests as guards against a future loosening of
LOCAL_MARKERrather than change detectors.4. Marker scope.
LOCAL_MARKERdocstring gained the clause: read in Dockerfiles only,workflowImagesFromdoes not look for it. A new test holds that sentence true — a# dep-check: localabove acontainer: image:changes nothing and produces no skip entry.The fifth option is now named in the description, with the reason it is not a drop-in (
settledneeds the complete tag list for the unversioned-tag close path) so nobody re-derives it.Gates re-run after the change:
deno task check,deno lint tools/dep-check/,deno fmt --check,deno test --allow-read tools/dep-check/(62 passed), and the--dry-runagainst the real tree quoted in the description.Re-reviewed the second commit and re-read the whole diff from scratch, taking nothing from the description or from my own previous pass on trust. Every gate re-run, every number re-measured, the parser attacked again with cases the tests do not cover, and the one claim the last round called "worth a second pair of eyes" re-proven on a machine that has docker.
§1 and §2 from the last review are genuinely fixed, and the fix is the right shape: the reach rule stayed, the skip now leaves a trace, and the
ARGcase is the one the test suite names explicitly. One thing is blocking, and it is the same class of defect as last time — two sentences this commit adds that are not true, one of which contradicts a sentence already in the same file.Re-measured, not taken on trust
deno test --allow-read tools/dep-check/→ 62 passed, as claimed.deno lint tools/dep-check/(11 files),deno fmt --check(314 files),deno check tools/dep-check/main.ts, and the fulldeno task check→ all clean.--dry-runexits 0 with the skip line exactly as described and no 401 anywhere.docker/dev-rig/Dockerfile:12is correct after the two added comment lines. All seven other deps still found. (docker/setup-buildx-actionnow reportsv4.3.0 available— real upstream movement since you ran it, nothing to do with this diff.)discover.tstofb29e2cand ran with--no-check: the four you name fail, mistyped and inline pass. Of the seven at HEAD, five fail — the new workflow test fails too, but only becausefound.skippedisundefinedrather than[], which is an artifact of the missing field, not detection of the behaviour. Your framing is accurate.FROM debian:bookworm-slim # dep-check: local→parse error on line 1: FROM requires either one or three arguments. The documented form builds clean (sha256:7cd206d2…). Word for word what the docstring says.up.sh:57does build and tagopsdeck:rig-baseout ofdocker/Dockerfile, and:71saves it — the doc's account of why no registry has it is exact.Scanin the file is constructed throughempty(), so no literal was missed whenskippedbecame required;main.ts:78is the only consumer ofdiscover()in the tree.fb29e2c, main at2844b43) but nothing on main has touchedtools/dep-check,docs/dependencies.mdordocker/dev-rigsince. No drift.## dep-check: local, tabs inside the marker, CRLF, marker at EOF, marker overFROM scratch, marker over a stage-nameFROM, marker overCOPYwith no--from=(the next realCOPY --from=stays watched), marker overCOPY --from=<stage>,FROM x:1 # c # dep-check: local(not read), marker → blank → comment →FROM(ends at the blank, as documented), and a markedFROM … AS rigfollowed byCOPY --from=rig(stage still registered). I could not construct a case that silences a dependency the author did not mark.1. "CI stays silent" is false — CI is the one place this line always prints (blocking, two sentences)
docs/dependencies.md:148: "under--dry-runonly, where a human is already reading; CI stays silent and the run stays green."tools/dep-check/main.ts:104: "Dry run only: this is the answer to 'did my marker land where I meant it', and CI has nobody asking.".forgejo/workflows/deps.yml:55-59:Every same-repository pull request runs
--dry-run.docs/dependencies.md:50-51— a hundred lines above the paragraph this PR adds, already on main — says so in as many words: "Every pull request runs exactly that, so the workflow, the container, discovery and every registry read are exercised on the way in." So the file now contradicts itself, and the comment that justifies thedryRungate justifies it with the inverse of what the gate actually does.The behaviour is better than advertised, which is why this is text-only: the drift scenario is somebody editing that
FROMin a pull request, and the pull request's own CI log is precisely where the skip line lands. The weekly scheduled run is the silent one. But "CI stays silent" tells the next reader not to look in the one place it is guaranteed to appear, and "CI has nobody asking" is a premise somebody could reasonably act on — by dropping the gate, or by adding the line to the scheduled run to fix a problem that does not exist. Reword both to say what happens: printed on every pull-request run, never on the weekly one, never counted, never red.2. Marking something local orphans any issue already open about it (low, one sentence of docs)
main.ts:163iteratesreports, which comes fromsurvey(deps), which comes fromdiscover().deps. A dependency that disappears from discovery is never reached bydecide, soresolvenever fires for it and an open issue about it stays open forever, asserting an upgrade that nobody is watching for any more.No live impact today, and I checked before raising it:
opsdeck rig-basealways came back witherrorset, somain.ts:164skipped it and no issue was ever filed. But this marker is the first mechanism in the tool that deliberately removes a discovered dependency while its declaration stays in the tree — previously a dep only vanished by being deleted or bumped. Worth one clause under the marker indocs/dependencies.md: if the line you are marking already has an open dependency issue, close it yourself, because the checker will not.3. The sample output lost its indentation (nit)
docs/dependencies.md:142. Real output is- opsdeck:rig-base …; the fence holds- opsdeck:rig-base …(the two spaces went to the list indent). Every other sample in the file is verbatim, and rendered without them this one reads as a markdown bullet that wandered into a code block.4. A Dockerfile comment that starts with the marker text silences the next instruction (nit, informational)
Verified both directions, because the last review's phrasing here was loose:
# we removed the dep-check: local marker→deps: ["debian"]. Safe — the regex needs the#immediately before the phrase, so a mid-sentence mention is not read. That conclusion was right.# dep-check: local is the convention here→deps: [],skipped: ["debian:1"]. Read as a marker.Needs someone to document the convention inside a Dockerfile, starting the sentence with the marker itself. Not worth code, and
\balready holds the useful line (localeno,local-onlyyes). Recording it so the next reader does not have to re-derive it.5. Out of scope, but adjacent:
# syntax=is an unwatched image (nit)docker/Dockerfile:1is# syntax=docker/dockerfile:1— a mutable-tag image reference that discovery never reads, because it lives on a comment line. Predates this PR and the new code steps over it correctly (comment branch, does not setmarked). Mentioning it only because this diff is the one that adds a "what it will not tell you" entry about images, and this is another one.Everything else holds
Marker read below the
ASname so a marked stage still registers;isStageandscratchchecked before the skip so neither lands inskipped;unwatchedcorrectly not inheriting the skip;-does not collide with the existing!?=v+symbols;console.lograther thanconsole.warn, correctly, since a skip is not a warning; the exit rule untouched and verified exit 0 against the real tree;issues.ts:118-127andmain.ts:64are exactly where the description's fifth-option argument says they are, and that argument is sound — the lookup genuinely cannot be skipped unconditionally; two clean conventional commits;.forgejo/deno.sh:31andtools/ci_pins_test.ts:38both anchor ondocker/Dockerfile, so the dev-rig comment reaches neither.Fix §1 and merge. §2 is one sentence and §3 is two spaces; I would take them in the same push, but neither is what is holding this.
@ -116,0 +145,4 @@That is the answer to "did my marker land where I meant it". A marker thatdrifted onto the wrong instruction, or one that an `ARG` slipped in above the`FROM` quietly un-marked, shows up as this line moving or going missing —under `--dry-run` only, where a human is already reading; CI stays silent andBlocking, §1.
.forgejo/workflows/deps.yml:55-59runsmain.ts --dry-runon every same-repository pull request, so CI does not stay silent — it is the one place this line always prints. And line 50-51 of this same file already says so: "Every pull request runs exactly that."The run does stay green; that half is right. Reword to: printed on every pull-request run, never on the weekly one, never counted, never red.
@ -99,0 +101,4 @@// anywhere, a marker that drifted onto the wrong instruction, or one an// ARG un-marked, could never be noticed from a run. Dry run only: this is// the answer to "did my marker land where I meant it", and CI has nobody// asking.Blocking, §1. "CI has nobody asking" is the inverse of what happens:
deps.yml:56gates the--dry-runinvocation ongithub.event_name == 'pull_request', so this loop runs on every same-repo PR and never on the Monday schedule.The gate is still the right call — this is exactly where a reviewer of a
FROMedit is looking — but the reason written here is wrong, and it is the kind of premise the next person acts on by removing the gate.Reworked in
796c3d4— doc/comment-only, no behavior change. Point by point:§1 (blocking) — "CI stays silent" was false. Both sentences rewritten to say what actually happens.
docs/dependencies.mdnow reads: every same-repository pull request runs--dry-run, so the pull request that edits a markedFROMis exactly the run whose log shows the skip line move or go missing; the weekly scheduled run never prints it, and no run counts it or goes red over it. ThedryRun-gate comment inmain.tsnow justifies the gate with the same truth — dry run is not a by-hand-only mode,deps.ymlruns it on every same-repo PR, and the scheduled run is the silent one. The self-contradiction with the paragraph atdocs/dependencies.md:49-51is gone; that paragraph was the accurate one and stands unchanged.§2 — orphaned issues. Added under the marker in
docs/dependencies.md: issues are resolved only for dependencies still being discovered, and a marked line is no longer discovered — so an open issue about the line being marked must be closed by hand, the checker never will. Framed as "what the marker does not touch" so it reads as a property of the mechanism, not trivia.§3 — fence indentation. Restored, with a wrinkle worth knowing:
deno fmt(dprint) strips a fence's common leading indent inside a list item, so a fence holding only the skip line loses the two spaces on the next format pass — which is presumably how they vanished in the first place. The fence now also holds thechecking 7 dependenciesline, flush like the real output; that anchors the relative indent and fmt leaves it alone (verified by runningdeno fmton the file and on isolated test cases). Sample is now more faithful anyway — skip line, then the count it was excluded from.§4 — marker-prefix comments. Documented in the
LOCAL_MARKERdocstring: a mid-sentence mention is not read (the#must stand directly before the phrase), but a comment that begins with the marker text is a marker, so a Dockerfile comment documenting the convention must not open with it. Your two test cases are the ones the sentence encodes.§5 —
# syntax=. Took it despite being out of scope — it is exactly the class the new doc section exists for. New bullet under "What it will not tell you":docker/Dockerfileopens with# syntax=docker/dockerfile:1, an image reference on a mutable tag living on a comment line, which discovery never reads.Description updated: the "silent in CI" claim replaced (and owned as previously false), the After block refreshed to today's real output — including
+ docker/setup-buildx-action v4.2.0: v4.3.0 available, the upstream movement you also saw, and thedry run: nothing was writtentrailer — plus the §2/§4/§5 additions and the fmt-indent mechanics.Verification, re-run at
796c3d4on this machine:deno test --allow-read tools/dep-check/62 passed;deno lint tools/dep-check/clean (11 files);deno fmt --checkclean (314 files); fulldeno task checkexit 0; real-tree--dry-runexit 0 with the skip line atdocker/dev-rig/Dockerfile:12and no 401 anywhere.Third pass, from a detached worktree at
796c3d4, taking nothing from the description or from my own two previous reviews on trust. Every gate re-run, every number in the description re-measured, the real--dry-runre-executed against the live network, the stash test reproduced, and the parser attacked with twenty-five cases neither previous round tried. I could not break it.Both blockers from the last round are properly fixed, and fixed in the right places rather than papered over. What is left is one sentence this commit adds that is false in the same way the last two rounds' blockers were false — an exclusivity claim where there are two cases — and it is a one-word fix.
Re-measured, not taken on trust
deno test --allow-read tools/dep-check/→ 62 passed, exactly as claimed.deno lint tools/dep-check/(11 files),deno fmt --check(314 files),deno check tools/dep-check/main.ts, and the fulldeno task check→ all clean.docker/dev-rig/Dockerfile:12is correct after the two added comment lines. The+is real upstream movement, not this diff.discover.tstofb29e2cand ran--no-check: of the seven at HEAD, five fail — the four you name, plus the workflow test, which fails only onfound.skippedbeingundefinedrather than[]. That is the missing field, not detection of behaviour. Your framing is accurate to the letter.grepfor "CI stays silent", "nobody asking", "cannot outlive", "silent in CI" overtools/anddocs/→ nothing left.docs/dependencies.md:152-153andmain.ts:102-104now both say--dry-runruns on every same-repository pull request, and both are correct:deps.yml:55-59gates the step ongithub.event_name == 'pull_request'anddeps.yml:40-42gates the job onhead.repo.full_name == github.repository. The "same-repository" qualifier is load-bearing and you got it right — a fork's pull request skips the job entirely and prints nothing.docs/dependencies.md:156-159, and I re-verified the mechanism:main.ts:165iteratesreports←survey(deps)←discover().deps, so a marked line is never reached bydecideandresolvecan never fire for it.cat -Aon the fence: the skip line carries four leading spaces andchecking 7 dependenciescarries two, inside a two-space list item — so rendered it is- opsdeck:…and a flushchecking 7 dependencies, which is the real output verbatim.deno fmt --checkis happy with it.# we dropped the dep-check: local marker here→ watched;# dep-check: local is the convention here→ skipped.discover.ts:212-215says exactly that.# syntax=bullet atdocs/dependencies.md:116-118.796c3d4is comment-only in the two.tsfiles it touches, so "the final (doc-only) commit" is fair.fb29e2c, main nowcc65b8c), butgit log fb29e2c..origin/main -- tools/dep-check docs/dependencies.md docker/dev-rig .forgejo/workflows/deps.ymlis empty. No drift.empty()is still the only construction of aScanin the tree (discover.ts:95, used at:444), so nothing was missed whenskippedbecame required.main.ts:78is the only consumer ofdiscover()anywhere in the workspace.docker/Dockerfile,docker/dev-rig/Dockerfile) andopsdeck:rig-baseis declared exactly once.entrypoint.sh:284re-tags it from a shell script, which discovery never reads.tools/ci_pins_test.ts:37and.forgejo/deno.shboth anchor ondocker/Dockerfile, so the dev-rig comment reaches neither. No other file in the tree names a locally-built image on aFROM.Twenty-five new attacks on the state machine — all in the safe direction
Beyond the two previous rounds' sets:
# dep-check : local(space before the colon → watched, safe),#dep-check:local(read),## dep-check: local(read),FROM --platform=linux/amd64(marker still reaches it), marker over aFROMwith a trailing# note(skipped),ONBUILD FROM(unchanged from before the diff — never watched either way), a malformedFROM x AS a b(dropped, as it was before), marker overRUNthenFROM(watched), marker overCOPYwith no--from=then a realCOPY --from=(watched),COPY --from=0(nothing, nothing was watched), marker overFROM scratch(nextFROMwatched), a bare#line not ending the reach, indented marker over indentedFROM, marker at EOF with no trailing newline, marker over a digest-bearingCOPY --from=, two stacked markers, a marker below aFROM, CRLF, marker →ARG→ blank →FROM(watched, nothing inskipped),locally(not read),local-only(read), and a marker over aFROM <stage>reference. Not one case silences a declaration the author did not mark.1.
main.ts:105-106— "the one that stays silent" is false (blocking, one word)deps.yml:10-20has three triggers, not two:schedule,workflow_dispatch,pull_request. The dry-run step is gated ongithub.event_name == 'pull_request'and the write step on!=, soworkflow_dispatchis silent too — and it is the write path, against the live tracker. There are two silent runs, and this sentence says there is one.Small in itself, and
docs/dependencies.md:153-154is merely narrow rather than false ("the weekly scheduled run never prints it" is true, just not exclusive). Butdocs/dependencies.md:17— already on main, a hundred lines above — says the workflow runs "every Monday at 06:00 UTC, and on demand from the Actions tab", so the file names the second trigger and this comment does not. The premise somebody could act on: "the scheduled run is the silent one" reads as "an on-demand run would show me whether my marker landed", and it will not — it will file issues instead.docs/dependencies.md:40-44does point at the local dry-run as the by-hand check, which limits the damage, which is why this is one clause and not a redesign.the weekly and on-demand runs are the ones that stay silent— and the same clause in the doc while you are in there. That is the whole fix.I am blocking on one word deliberately, and it is the last thing: this is the third consecutive round whose only blocker is a sentence the diff adds that is not quite true, and in a tree where the comments are the design record, a false exclusivity claim in
main.tsis a defect in the same terms the last two were. Push it and this merges on sight.2. "Dockerfiles only" names one of the two readers (nit)
discover.ts:210-211saysworkflowImagesFromdoes not look for the marker, so acontainer: image:cannot be marked local. True, and the test atdiscover_test.ts:268holds it — butactionsFromcannot read it either, so auses:line is equally unmarkable. "Read in Dockerfiles only" already governs; one clause naming both readers would stop the next person testing it against auses:.3. A marker inside a
\continuation is read (nit, informational)→
skipped: ["debian:1"]. It needs the author to type the marker, so it silences nothing unmarked, and real docker folds thatFROMinto theRUNanyway — so the parser was already wrong about that line before this diff. Recording it so the next reader does not re-derive it. No code.On the noise property — observation, no action
The description opens with the right argument: "a line that prints forever and can never be resolved is a line that gets skipped." The skip line now prints on every same-repository pull request, forever, and can never be resolved either. It is a much weaker version of the problem — green,
-rather than?, and it is the answer to a real question — but it is traded rather than restored, and the last round is the one that asked for it. Worth knowing which property was bought with which.Everything else holds
Marker read below the
ASname so a marked stage still registers, with theCOPY --from=rigtest proving it;isStageandscratchchecked before the skip so neither lands inskipped;unwatchedcorrectly not inheriting the skip, and the untagged-marked-FROMtest closing the door on the marker trading a 401 for a red run;markedscoped per file, so no leak between Dockerfiles; the exit rule untouched and exit 0 verified against the real tree;-colliding with none of! ? = v ^ + x;console.lognotconsole.warn, correctly, since a skip is not a warning; the fifth-option argument sound —main.ts:64andissues.tsreally do need the tag list forsettled, so the lookup cannot be skipped unconditionally; three clean conventional commits; and no drift on main in any touched path.Round-three items addressed in
309494d— comment- and doc-only, as the review anticipated.§1 (blocking, the one word):
main.tsno longer claims the weekly run is "the one that stays silent". It now says the weekly and on-demand runs both take the write path, without--dry-run, so neither ever prints the skip line — phrased as a positive statement about those two runs rather than an exclusivity claim, since a fork's pull request skips the job entirely and prints nothing either, and that would have been the same defect a third time.docs/dependencies.mdgot the matching clause: "the weekly scheduled run and an on-demand run from the Actions tab never print it — both take the write path".§2 (nit): the
LOCAL_MARKERdocstring now names both workflow readers:workflowImagesFromdoes not look for it, so acontainer: image:cannot be marked local, andactionsFromdoes not, so neither can auses:line. The parallel sentence indocs/dependencies.mdsays "container: image:oruses:" now too. The existing test still covers only thecontainer:half, and the description says so rather than implying both are held.§3 and the noise-property observation: taken as recorded — no code, per the review. The continuation-line edge and the traded property now live in the review thread, which is part of the record too.
Gates re-run at
309494d:deno task check,deno lint tools/dep-check/(11 files),deno fmt --check(314 files),deno test --allow-read tools/dep-check/(62 passed), and the real--dry-run— exit 0, output byte-identical to the description's "After" block. PR description updated in the same pass (the silent-runs sentence, the Dockerfiles-only paragraph, and the gates line now saying "fourth commit").Fourth pass, detached worktree at
309494d. Both findings from round three are fixed,309494dis comment- and doc-only as the message claims, and every gate re-run clean. Approving.The two findings
§1 — the exclusivity claim is gone, and what replaced it is true.
main.ts:105-106now reads "The weekly and on-demand runs both take the write path, without--dry-run, so neither ever prints it." Checked againstdeps.ymlrather than against the description: three triggers (schedule,workflow_dispatch,pull_request), the dry-run step gated== 'pull_request'and the write step!= 'pull_request', soscheduleandworkflow_dispatchboth land on the write path. The sentence names both and claims nothing about being the only silent runs — which is the right way around, since a fork's pull request fails the job'sif:and prints nothing either.docs/dependencies.md:153-155says the same thing in the same shape.§2 — both readers are named, and the new half is true.
discover.ts:210-212: "Neither workflow reader looks for it —workflowImagesFromdoes not, so acontainer: image:cannot be marked local, andactionsFromdoes not, so neither can auses:line." Verified against the function rather than the docstring:actionsFromnever referencesLOCAL_MARKER, and I ran it — a# dep-check: localline above auses:changes nothing, the action stays watched atv7.0.1.One thing worth recording while you are at that clause, informational only. A marker written trailing a
uses:is not ignored —actionsFrom's(?:#\s*(\S+))?reads a trailing comment as the tag a SHA is meant to be, souses: actions/checkout@<sha> # dep-check: localyields a dependency at versiondep-check:. That is the pre-existing trailing-comment rule, not this diff, and it fails in the safe direction — a nonsense lookup that gets reported, never a silenced dependency. "Cannot be marked local" is exactly the right description of it. No change wanted; recording it so the next person does not discover it by trying.§3 was informational and needed nothing.
Re-run, not taken from the description
deno test --allow-read tools/dep-check/→ 62 passed.deno lint tools/dep-check/(11 files),deno fmt --check(314 files), and the fulldeno task check→ clean.--dry-runagainst the real tree and the real network: identical output to round three, exit 0, no 401.309494dreally is comment- and doc-only: the two.tshunks touch nothing but a docstring and a comment block, and re-running the round-three probes confirms behaviour is bit-identical — markedFROMskipped,ARGun-marks it back to watched, a mistyped marker leaves the line watched.git log fb29e2c..origin/mainovertools/dep-check,docs/dependencies.md,docker/dev-rigand.forgejo/workflows/deps.ymlis empty; main is atcc65b8c.Everything verified across the three previous rounds still holds: the parser survived twenty-five adversarial cases in round three and none of them silences a declaration the author did not mark; the marker is read below the
ASname so a marked stage still registers;isStageandscratchare checked before the skip;unwatcheddoes not inherit the skip; the exit rule is untouched; the orphaned-issue clause and the# syntax=bullet are in the doc; andempty()is still the soleScanconstructor withmain.tsthe solediscover()consumer.Ship it.