Make linkcheck robust to bot-blocking instead of denylisting domains - #82
Open
fedorov wants to merge 2 commits into
Open
Make linkcheck robust to bot-blocking instead of denylisting domains#82fedorov wants to merge 2 commits into
fedorov wants to merge 2 commits into
Conversation
linkcheck has been flaky: three consecutive runs on effectively
identical content gave green, red, green, with a different set of
third-party sites failing each time — dicom.offis.de, peerj.com and
support.dcmtk.org in one run; registry.opendata.aws,
nci-crdc.datacommons.io and huggingface.co in the next. Every one of
those URLs returns 200 from an ordinary client. The sites are
intermittently blocking GitHub Actions' datacenter IP ranges, and
huggingface.co returned 429 Too Many Requests, which is lychee's own
concurrency tripping a rate limit.
Because fail: true gates PRs, this produces false alarms that are
indistinguishable from real breakage.
Tune the checker rather than growing the denylist:
--max-concurrency 8 down from lychee's default of 128; the 429
was self-inflicted
--max-retries 5 up from 3
--retry-wait-time 5 up from 1s, so retries survive a rate-limit
window instead of hammering through it
--timeout 30 up from 20s
--accept ...,403,429 403 and 429 mean "we were refused", not
"the link is dead"; a genuinely broken link
returns 404, which still fails
Note --accept replaces lychee's default rather than extending it, so
the success range 100..=103,200..=299 is restated explicitly.
This supersedes the four domain excludes added alongside the AI
assistants docs (claude.ai, dicom.offis.de, peerj.com,
support.dcmtk.org), so remove them. Those URLs are now checked again
and will still fail on a 404 — strictly better than skipping them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With 403/429 now accepted, most of the existing --exclude entries were
suppressing links that are perfectly checkable. The broadest,
'cancer.gov', was hiding 132 IDC-owned URLs — every link to our own
portal and API — so a dead portal deep-link could never fail CI.
Probed every excluded domain's actual URLs before deciding. Dropped:
cancer.gov 132 IDC URLs + NCI pages; portal 200,
api 200 (see 406 note below)
doi.org 282 URLs, all resolving
cloud.google.com \ 107 URLs between them, all 200
console.cloud.google.com/
dicom.nema.org 200 once unescaped; the 404s were an
artifact of markdown underscore-escaping
scholar.google.com 200 (403 from datacenter IPs is now accepted)
mayo.edu 403, now accepted
access-ci.org 200
towardsai.net 200
Kept, with the reason recorded in a comment above the step:
localhost not reachable from CI by definition
linkedin.com serves HTTP 999 to datacenter IPs, outside
any sane accept range
storage.googleapis.com sole occurrence is inside an HTML <a> in a
fenced code block, extracted malformed
docs.google.com one embedded doc returns 401, i.e. it is
login-gated — a real content problem to fix,
not something to paper over
viewer.imaging... SPA; deep links 404 to a plain HTTP client
proxy.imaging... quota-limited proxy
Also extend --accept with 405 and 406. The documented MCP endpoint,
https://api.imaging.datacommons.cancer.gov/mcp, answers a bare GET with
406 because it requires MCP's Accept header. Like 403, that means the
endpoint exists but will not serve a bot — as opposed to 404/410, which
still fail.
Net effect: 532 more URLs actually checked, 135 of them IDC-owned.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
linkcheckis flaky. Three consecutive runs on effectively identical content gave green → red → green, with a different set of third-party sites failing each time:dicom.offis.de,peerj.com,support.dcmtk.org— all403registry.opendata.aws,nci-crdc.datacommons.io—403;huggingface.co—429 Too Many RequestsEvery one of those URLs returns 200 from an ordinary client, including with lychee's own user-agent — so this is not user-agent sniffing. The sites are intermittently blocking GitHub Actions' datacenter IP ranges. The
huggingface.co429is different in kind: that one is self-inflicted, lychee's default concurrency of 128 tripping a rate limit.Because
fail: truegates PRs, this produces false alarms that are indistinguishable from real breakage — and the standing workaround (add another domain to--exclude) permanently stops checking those links, including for genuine 404s.The fix
Tune the checker instead of growing the denylist:
--max-concurrency8429was self-inflicted--max-retries5--retry-wait-time5--timeout30--accept100..=103,200..=299(default)100..=103,200..=299,403,429403/429mean "we were refused", not "the link is dead"403and429are refusals to answer, not evidence of breakage. A genuinely dead link returns 404, which still fails the build.Denylist cleanup
This supersedes the four domain excludes added alongside the AI assistants docs in #81 —
claude.ai,dicom.offis.de,peerj.com,support.dcmtk.org— so they are removed here. Those URLs are now checked again and will still fail on a404, which is strictly better than skipping them entirely.The older excludes (
doi.org,cancer.gov,mayo.edu, …) are left alone — they predate this and may have been added for other reasons. Some are probably now redundant too and could be revisited separately.🤖 Generated with Claude Code