Audit the documentation links with the organisation's check-links action - #1123
Draft
notluquis wants to merge 16 commits into
Draft
Audit the documentation links with the organisation's check-links action#1123notluquis wants to merge 16 commits into
notluquis wants to merge 16 commits into
Conversation
Nothing verified that the links in the markdown files still resolve. The run takes about three seconds and needs no install, since it reads the markdown rather than the built app. Report only, the same stance zizmor.yml took when it landed, so an outage on someone else's host does not block a pull request that changed nothing. The baseline is clean, so making it a gate later is a one-line change. blog.jupyter.org is excluded: it is on Medium, which answers the checker with 403 while the pages are alive. That was measured rather than assumed, on a run with the exclusion removed, where all six of those came back 403 and nothing else did. Wrote this with Claude Code. I ran the workflow on a branch in my fork before opening this, which is also how I found that a comment inside the folded args block gets passed to lychee as an argument, and that report-only hides the resulting usage error as a passing job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
The comment claimed lychee resolves github.com links through the API with this token. That is plausible and was not checked, and the reason for the grant is just that a token is passed at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
Neither the jupyterlab nor the jupyter organisation runs lychee anywhere. Nine repositories across the two run pytest-check-links, and the organisation publishes jupyterlab/maintainer-tools/.github/actions/check-links for it, which is what jupyterlab/jupyterlab calls from check-external-links.yml. A repository local link checker duplicates both that action and the shared ignore list it carries. Its ignore list already covers blog.jupyter.org, for the same reason measured here: those pages are on Medium, which answers a checker with 403 while loading fine in a browser. That is the one entry this workflow passes, since the defaults already skip pull request and issue URLs, docs.github.com, npmjs and localhost. The trigger comes from the same workflow: a pull request that changed no line containing a URL skips the audit. Filtering on paths instead would miss a link added to a .ts or a .yml and would still run on a markdown change that touched no link. Unlike the lychee setup this replaces, the action fails the build on a dead link. That is how the rest of the organisation runs it, and the baseline here is clean. Worked through this with Claude Code. I ran the workflow on a branch in my fork: the audit job walked all seven markdown files and passed with only that one ignore entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
notluquis
changed the title
Three things a review of this branch turned up. The checker validates a link with a full GET, no HEAD and no streaming, so the five installer links in the README were downloaded in full on every run and written to the requests cache. Measured against v4.6.2-1: 600 + 380 + 409 + 424 + 411 MB, 2.2 GB per run, which is the same weight jupyterlab#1072 is about seen from a different side. Ignoring github.com is what jupyterlab's own list does, and it also keeps thirty unauthenticated fetches away from the anonymous rate limit, which matters because this check blocks. The seventeen external links that are not on github are still audited, and the audit job went from 58 to 27 seconds. The change detection ran on any diff containing a URL, so yarn.lock alone would have made every dependency bump audit documentation it never touched. It is now scoped to the file types the checker reads. The fallback for an unreachable pull request SHA had been dropped. Without it git exits 128 and the check goes red for a reason unrelated to any link. Worked through this with Claude Code. I measured the installer sizes myself, confirmed the checker does an unstreamed GET by reading its plugin source, and ran the workflow again on my fork: both jobs pass and the audit walks all seven markdown files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
The concurrency group keyed on the ref alone, so a push to master would cancel a running weekly audit. A cancelled run is grey rather than red, so the skipped week would pass unnoticed. Only a superseded pull request run is worth cancelling. The comment justifying the URL gate said a `paths:` filter would miss a link added to a .ts or a .yml. That is wrong: check_links.py globs only md, rst and ipynb, so a link in a .ts is never audited either way. The real justification is the other half, that a markdown change touching no link has nothing to audit and `paths:` cannot tell the two apart. The inventory of what stays audited was counted on the documentation branch rather than on master, so it named hosts this branch does not link and put the total at seventeen. On master it is nine links across six hosts. Two gaps are now written down rather than left to be rediscovered: a rename of something a document points at changes no line containing a URL, so it passes the pull request and surfaces on the push run; and Python's glob skips dot-directories, so nothing under .github is ever audited. Second code review pass over the branch. Every number above was recounted against master rather than corrected in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
A third review pass made the case for deleting the job rather than fixing it. jupyterlab puts it in front of the audit because theirs builds the docs first and takes minutes; here the audit is 25 seconds, so a second runner and a full-history clone on every pull request cost more than they save. It was also the wrong gate. Counted over the seven audited files as the checker sees them, rendered anchors rather than text, 65 of the links are relative or local against 8 external ones. A broken relative link changes no line containing a URL, so gating on `https?://` skipped the majority case and let it surface on master instead. Deleting the job closes that and removes the reason this pull request could not exercise its own check. Three numbers in the comment were wrong and are recounted: 18 github.com links rather than 30, 8 audited across 5 hosts rather than nine, and nodejs.org named although its URL is bare text and never becomes an anchor. Worked through this with Claude Code. The counts come from rendering each file's anchors rather than grepping its text, which is what produced the wrong ones the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016CxXVgkyGjZcrtvCa8WudF
Four em dashes had gone into the comments, which is not how anything else in this repository is written. The block was also half the file: 38 comment lines out of 66, against 14 to 28 per cent for every workflow already on master. Most of it was the pull request body pasted into a config file, counts and known gaps included. Those belong in the pull request, where they are read once by a reviewer, rather than in a file where they go stale unwatched. What stays is the part a reader of the file needs and cannot derive: why two hosts are ignored, and why there is no change-detection job. Behaviour is unchanged; the diff is comments only.
The comments in this file were hand-wrapped, which is not how this repository writes them: one comment on one line, however long it runs, so that editing a sentence does not rewrap the paragraph and hide the change. Behaviour is unchanged; the diff is line breaks.
notluquis
marked this pull request as ready for review
August 24, 2026 01:57
…mments
Three findings from a review pass, all confirmed against the tree first.
The audit blocked every pull request on hosts belonging to other people. One
403 from Cloudflare or a Read the Docs outage would have turned a TypeScript
change that touches no documentation red, with the fix being an edit to this
file under pressure. It is advisory on a pull request now and still a hard gate
on master and on the weekly run, where nothing is waiting on it and link rot
surfaces the same. The repository already reaches for this shape in zizmor.yml,
though for the opposite reason: there it is a backlog to clear, here the
flakiness belongs to somebody else's server and does not clear.
The concurrency comment credited `cancel-in-progress` with what the group key
already does. `github.event_name` is in the group, so push, schedule and
pull_request were never able to cancel each other; the conditional narrows it to
a superseded run of the same pull request. Behaviour was right, the explanation
was not, and in a file whose comments are the only documentation that is what
the next person acts on.
The body said a gap was written into the file and it was not. `check_links.py`
collects with `glob.glob('**/*.md', recursive=True)`, which skips
dot-directories, so `.github/` markdown is outside the audit permanently. Read
the pinned action rather than assumed, and measured both globs, because
`pathlib.Path.glob` does see those files: reading the wrong one of the two is
how this looks covered when it is not.
zizmor is clean on the result and the YAML parses. Worked through this with
Claude Code; I read the action's source to settle the glob question rather than
taking either the review's word or my own first test, which used the wrong glob.
…ecays diff-hygiene read the two adjacent comments as one hand-wrapped block; they are two ideas and a blank line says so. The first also asserted the audit is 25 seconds, and five real runs give 10 to 37, so the figure is now the range's meaning rather than a number: what the argument uses is that it is quick enough that a second runner costs more than it saves.
The last commit put `continue-on-error` on the audit so a third-party outage would not redden an unrelated pull request. A review pass pointed out what that actually does: the step fails and the job still concludes success, so `gh pr checks 1123` prints `pass` either way and the only surface is the raw log. That is the same criticism this repository already writes about zizmor, and `ready-check.mjs` names any check whose workflow carries the key for exactly this reason. It would have been the second one. So the trigger goes instead. Neither shape of it worked: blocking, a 403 from Cloudflare reddens a TypeScript change that touches no documentation; advisory, nobody sees the failure. What is left is that a link added in a pull request is checked on the push that merges it, minutes later, on a run where nothing is waiting on the answer. Link rot over time, which is what the weekly cron is for, is unaffected either way. The concurrency block follows: with no pull_request event there is nothing that supersedes anything, so it cancels nothing rather than conditionally. zizmor is clean, the YAML parses, and no step carries `continue-on-error` any more. Worked through this with Claude Code; I checked `gh pr checks` on this pull request to confirm a `continue-on-error` step really does report pass.
notluquis
marked this pull request as draft
August 25, 2026 16:58
…nterpreter Four findings from a review pass, each checked against the tree first. The `https://github.com/.*` entry silenced twenty links to spare five. Counted on master: twenty-five github.com links in the root markdown, of which five are the `releases/latest/download/` installers carrying the 2.2 GB the entry exists for. The other twenty cost nothing to check and include a file path inside somebody else's repository, which is exactly the shape a rename breaks, and six workflow links of the kind jupyterlab#1121 just moved. jupyterlab ignores the host outright because their docs carry hundreds; here the narrow pattern keeps twenty audited at no measured cost. `setup-python` ahead of the action, because it runs a bare `pip install pytest-check-links[cache]` with no interpreter of its own and would otherwise use whatever the runner image ships. `sync_lab_release.yml` already pins one for the same reason, and this uses that same SHA rather than a different version of the same action. A reinstated PEP 668 marker or an image bump would otherwise redden a hard-gating master check for a reason unrelated to link rot. The `# v1` label is not what `v1` resolves to any more, and the comment now says so. Not changed, because `jupyterlab/jupyterlab` pins this exact SHA with this exact label, and diverging from an identical upstream line to correct a cosmetic tag is churn a reviewer pays for. The action's own files are unchanged between the pinned commit and current `v1`. zizmor is clean and the YAML parses. Worked through this with Claude Code. I first wrote a `setup-python` SHA from memory, checked it against the API, and replaced it with the one this repository already uses.
…comments The ignore pattern matched any repository's latest-download URL rather than this one's, and is anchored now. It also left the five most used links in the repository unaudited by construction, which is the shape a packaging change breaks: those filenames come from electron-builder's `artifactName`, so a rename puts a 404 on every install instruction with the one workflow meant to catch dead links unable to see it. A HEAD check covers them, reading the list out of `README.md` rather than carrying its own copy, and failing if it finds none, because a pattern and a table that drift apart would otherwise pass silently. Run against the real URLs: five found, all 200. Three comments said things the file does not do. `cancel-in-progress: false` does not keep a pending run. GitHub's workflow syntax reference says an existing pending run in the group is cancelled when a new one arrives, and holding them needs `queue: max`. What actually stops a push from cancelling the weekly run is `github.event_name` in the group key, so the comment now credits that. "A hard gate" overstated it. Without a `pull_request` trigger there is no check to block a merge: it reddens the commit and the scheduled run, which is still worth having and is not gating. zizmor is clean and the YAML parses. Worked through this with Claude Code. The HEAD loop reported failures on its first run, which turned out to be leftover state in my own shell rather than the step: in a clean subshell all five answer 200, and that is what the committed version was checked against.
diff-hygiene reported it and the push went out first, which is the third time today; the Stop hook written for exactly this would have caught it at the end of the turn.
…ly weaker All four are in the step added one commit ago, and a review pass found them. The drift guard could not run. `set -o pipefail` makes `grep | sort` carry grep's exit 1 when nothing matches, and `set -e` then aborts at the assignment before the check below it, so the case the guard exists for produced a red step with an empty log instead of saying the pattern and the documentation had drifted apart. `|| true` on the assignment, and the message prints: verified both ways. The step ran only after success, which is when it is least needed. A 403 from a newly added host, or any unrelated failure in the audit above, skipped the installer check entirely, so the coverage the pull request body promises disappeared exactly when something was broken. `if: always()`. `|| echo 000` concatenated onto curl's own output rather than replacing it, because curl writes the `-w` value even when it exits non-zero. Measured: `[000000]`, not `[000]`. Harmless there, but `2*` matches `200000`, so a request that emitted a 2xx and then failed on a later hop would have been scored healthy. `|| code=000` on the assignment and `2[0-9][0-9]` instead of the prefix glob. The ignore covers every markdown file the action walks and the check read `README.md` alone, so an installer link added to `user-guide.md` would be both ignored and unchecked. It greps the same set now. Verified on the real URLs: five found, all 200; a deliberately dead one under the same path returns 404 and fails the step; the drift path prints its message. zizmor is clean. Worked through this with Claude Code. I had told the user the `000000` I saw earlier was leftover state in my shell. It was not: it is this bug, and the measurement above is what showed it.
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
1 participant
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.
References
jupyterlab/maintainer-tools/.github/actions/check-links, the actionjupyterlab/jupyterlabcalls fromcheck-external-links.yml.Code changes
None. One new workflow,
.github/workflows/check-links.yml, one job.The action wraps
pytest-check-links, which is what this ecosystem uses: nine repositories acrossjupyterlabandjupyterrun it, and neither organisation runs anything else. Using the shared action means its defaults come with it rather than being reproduced here.It runs on push to master, on a weekly schedule and on manual dispatch, and deliberately not on pull requests. The schedule is the one that matters, since link rot happens over time rather than per commit; a link added in a pull request is checked on the push that merges it, minutes later, on a run where nothing is waiting on the answer.
No change-detection job. jupyterlab puts one in front of the audit that skips it when no changed line contains a URL, which earns its second runner there because their audit builds the docs first and takes minutes. Here the audit is 25 seconds, so the gate would cost more than it saves. It would also be the wrong gate: most of the links in these files are relative rather than external, and a broken relative link changes no line containing a URL. Gating on
https?://would skip the majority case and let it surface on master instead.ignore_linkspasses two entries, narrowed to hosts this repository actually links rather than copied whole:https://github.com/jupyterlab/jupyterlab-desktop/releases/latest/download/.*. The checker validates a link with a fullsession.get(), no HEAD and no streaming, so those five download in full every run and are written to the requests cache. Measured against v4.6.2-1: 600 + 380 + 409 + 424 + 411 MB, 2.2 GB per run. The 2.2 GB carries the entry on its own. Narrowed to this repository's release path rather than the whole host, which jupyterlab ignores because their docs carry hundreds of github.com links: here the other twenty cost nothing to check and include a file path inside somebody else's repository, the shape a rename breaks. A separate step covers the five with a HEAD request, so ignoring them here does not leave the most used links in the repository unaudited.https://blog.jupyter.org/.*. Those pages are on Medium, which answers a checker with 403 while loading fine in a browser. Measured on a run without it: all six came back 403 and nothing else did. jupyterlab's list carries this one too.What stays audited: every relative link in the documentation and every external link except the two entries above, github.com's other links included.
docs.conda.iois on jupyterlab's ignore list and not on this one, because both of its links answered 200 when measured; if it turns flaky it belongs here. Worth saying about the weekly run, which is the leg this exists for: GitHub sends a scheduled workflow's failure notification only to whoever last edited the cron line, so a link that dies months from now reddens a run with no pull request and no commit anyone is watching. Someone has to own that or it wants a step that opens an issue on failure; this pull request does neither.One gap is written into the file rather than left to be rediscovered:
check_links.pyglobs**/*.md, and Python's glob skips dot-directories, so nothing under.githubis audited, neither the pull request template nor the issue template.The action fails the build on a dead link and has no report-only mode. That is how the rest of the organisation runs it.
User-facing changes
None.
Backwards-incompatible changes
None.
Manual testing
Not exercised on this pull request, because there is no
pull_requesttrigger andworkflow_dispatchneeds the file on the default branch first: the first run of this shape will be the push that merges it. Earlier shapes, including the one with the pull request trigger, ran seven times on a branch in my fork with a temporary trigger. Both the two-job and the one-job shapes pass; the audit walks every markdown file at the repository root in well under a minute. Before the github.com entry the same job took 58 seconds, which is the 2.2 GB.Measurements behind the two ignore entries, all made rather than assumed:
content-lengthon the five installer URLs for the sizes; a run with the Medium entry removed, where all six of those came back 403; readingpytest_check_links/plugin.py, which callssession.get(url_no_anchor)with nostream, for why a link check downloads anything; and rendering each file's anchors rather than grepping its text for the counts, which is what produced the wrong ones in an earlier revision.zizmorreports no findings on the new file, checked locally with the versionzizmor.ymlpins.Three code review passes over the branch. The third is what argued for deleting the job rather than fixing it.
AI usage