Skip to content

fix(lychee): accept 429 and exclude four checker-blocked hosts - #303

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/lychee-accept-429-and-bot-blocked-hosts
Jul 30, 2026
Merged

fix(lychee): accept 429 and exclude four checker-blocked hosts#303
kyle-sexton merged 2 commits into
mainfrom
fix/lychee-accept-429-and-bot-blocked-hosts

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

melodic-software/claude-code-plugins' rolling link-check report
(ccp#640) listed 8 errors.
Three were genuinely dead links and are fixed in that repo. Five were not link defects at all
every one is a live URL the checker cannot reach — and lychee is a managed component for that
repo, so their fix belongs here rather than in a materialization that the next sync would overwrite.

Every URL below was verified alive before being handled, and each fix is the narrowest one that
matches the actual cause.

429 is accepted, and no rate-limited host is excluded

A 429 means the server is rate-limiting the checker — never that the link is dead. It also lands
on whichever host the shared CI runner IP happens to be throttled against on a given run, so it
names a different healthy URL each time. Excluding the host that happened to lose that lottery
(www.gnu.org/software/coreutils/..., verified 200) would stop checking a link that is fine and let
the next run throttle a different one, so accept carries 429 instead.

The range spells out lychee's own documented default rather than only listing 429:

accept = ["100..=103", "200..=299", "429"]

accept replaces the default set, it does not extend it — so writing the default back is what
keeps this from silently narrowing what counts as success. The default was read from the pinned
binary's own --accept help ([default: 100..=103,200..=299]), not recalled.

Four hosts excluded, all checker-side limitations

Host Reported as Verified
dl.acm.org (DOI) 403 403 even with a full browser User-Agent — no header tuning reaches it
queue.acm.org 403 same
docs.genius.com 403 200 with a browser User-Agent — the documented bot-block case this list exists for
www.ntia.gov (SBOM PDF) SSL certificate not trusted chain verifies locally: openssl s_client returns Verify return code: 0 (ok) against a Cloudflare TLS Issuing ECC CA intermediate, and curl fetches it 200 under strict verification

The NTIA entry is the one worth a second look on review: the report's wording ("SSL certificate not
trusted") reads like a security signal, but the chain is fine off the runner, so this is a trust
store failing to complete an ECC chain rather than an untrustworthy host. It is recorded that way in
the file, with a note to re-test before removing — a genuinely bad chain would fail locally too.

The list header now says plainly what the list is: checker-side limitations on links verified to
be alive, never a dead link silenced
, and each entry records the verification that earned it so a
future reader can re-test rather than trust the list.

One test-shape correction

The Genius assertion is a line-anchored assert_row_count, not assert_not_contains. The excluded
URL is the host root, so every sibling path that must stay checked (docs.genius.com/example)
contains it as a prefix — assert_not_contains could only have passed by deleting the sibling
control that proves the boundary, which is the assertion doing the opposite of its job. The other
three new exclusions have no such collision and use the existing idiom unchanged.

Verification

  • bash components/lychee/lychee.test.sh53 assertions, 0 failures, exit 0 (was 45; 8 added:
    one exclusion proof and one still-checked sibling per new host)
  • Each new exclusion is paired with a sibling-path control, matching how every existing entry in
    this list is tested
  • shellcheck components/lychee/lychee.test.sh — clean
  • markdownlint-cli2, typos, gitleaks, editorconfig — all clean via the pre-commit hook
  • lychee.toml parses as valid TOML; accept and the 20 exclude entries read back as intended

Related

  • No linked issue.
  • ccp#640 — the link-check
    report that surfaced these five; its three genuine dead links are fixed separately in that repo
  • distribution/sync-manifest.ymllychee is managed for melodic-software/claude-code-plugins
    (and for .github, ci-runner, ci-workflows, dotfiles), so this change reaches them through
    the normal sync

🤖 Generated with Claude Code

https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C

Five of the eight errors on claude-code-plugins' rolling link-check report were
not dead links. Each was verified alive before being handled here, and each fix
is the narrowest one that matches the actual cause.

A 429 is the SERVER rate-limiting the checker, never evidence a link is dead,
and it lands on whichever host the shared CI runner IP happens to be throttled
against on a given run — so it names a different healthy URL each time. The
accepted-status range now carries 429. The range restates lychee's own
documented default (`--accept` help: `[default: 100..=103,200..=299]`, read from
the pinned binary rather than recalled) because `accept` REPLACES the default
rather than extending it, so spelling it out is what keeps the change from
silently narrowing what counts as success. Excluding each rate-limited host was
rejected: it stops checking a link that is fine, and the next run throttles a
different one.

Four hosts are excluded, all of them checker-side limitations on links proven
to be alive:

- dl.acm.org and queue.acm.org answer 403 even with a full browser User-Agent,
  so no header tuning reaches them. A DOI is also the most stable citation form
  there is.
- docs.genius.com answers 403 to the checker and 200 with a browser User-Agent —
  the documented bot-block case this list already exists for.
- www.ntia.gov is reported as "SSL certificate not trusted", but the chain
  verifies locally: `openssl s_client` returns `Verify return code: 0 (ok)`
  against a Cloudflare TLS Issuing ECC CA intermediate, and curl fetches it 200
  under strict verification. That is a trust store failing to complete an ECC
  chain, not an untrustworthy host.

Every entry records the verification that earned it so a future reader can
re-test instead of trusting the list, and the header now says plainly that this
list only ever holds live links the checker cannot reach.

The Genius assertion is a line-anchored row count rather than a substring
absence. The excluded URL is the host root, so every sibling path that must stay
checked contains it as a prefix — assert_not_contains could only pass by
deleting the sibling control that proves the boundary.

No linked issue.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C
@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 2s —— View job


Claude finished the code review

  • Gather context (diff, changed files, CLAUDE.md/REVIEW.md)
  • Understand the request
  • Review lychee.toml changes
  • Review lychee.test.sh changes
  • Review fixtures/good/Exclusions.md changes
  • Post final review

Summary

This is a well-scoped, well-verified fix: it widens accept to the checker's own documented default plus 429 (never per-host, so the throttle lottery doesn't just move to a different exclude entry), adds four narrowly-anchored exact-URL excludes for checker-side limitations (two ACM 403s, a Genius bot-block, and an NTIA TLS-chain gap), and corrects the Genius assertion from assert_not_contains to a line-anchored assert_row_count — a real bug fix, since the excluded URL is the host root and every sibling path contains it as a prefix. Every new exclusion is paired with a sibling "still checked" control, matching the existing idiom. I couldn't execute lychee/the test script myself (sandboxed, no approval available for those commands), so this is a static review — the PR's own reported run (53 assertions, 0 failures) is consistent with what I traced through by hand.

Two minor, non-blocking nits:

1. Regex suffix inconsistency across the four new excludeslychee.toml#L43-L54

The dl.acm.org (L43) and docs.genius.com (L47) entries end in /?([?#].*)?$, matching the established idiom used by every pre-existing exact-URL exclude in this list (medium.com, mysql, miro, npmjs, w3.org). The queue.acm.org (L44) and www.ntia.gov (L54) entries instead end the pattern hard at the literal path/query, with no optional trailing slash, query, or fragment allowed. Functionally this still matches today's verified-alive URL, and it's arguably in the spirit of "narrowest fix that matches the actual cause" from the PR description — but if either site ever appends a tracking query string or the checker normalizes a trailing slash, the exclude silently stops matching and reintroduces the exact false-positive report this PR fixes. Worth a quick pass to make the four new entries consistent with each other and the rest of the list (or a one-line comment explaining why these two are intentionally stricter, if that was deliberate).

2. README.md doesn't mention the new exclusion categorycomponents/lychee/README.md#L18-L24

The component README describes the online lane's exclude entries in two buckets: private-repo/auth-boundary URLs, and "public bot-blocking exceptions ... scoped to the exact URL, not the host." The NTIA entry this PR adds is neither — it's a checker/runner TLS trust-store gap, not a bot block (the PR body itself flags this as "the one worth a second look on review" for exactly that reason). lychee.toml's own inline comment was updated to describe this new category accurately, but the README's summary wasn't, so a reader who trusts the README's framing (as the file itself instructs: "read it there rather than from a copy here") would miss why an NTIA-style entry exists. A one-line addition to the README's bucket list would keep the two in sync — not part of this PR's stated scope, so raising it rather than blocking on it.

Everything else — the accept range rationale, the 403-vs-bot-block verification methodology, the ACM DOI/queue exclusions, and the assert_row_count fix — checks out as sound and consistent with the file's existing conventions.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e418063612

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread lychee.toml
@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

kyle-sexton added a commit to melodic-software/claude-code-plugins that referenced this pull request Jul 30, 2026
The rolling link-check report listed **8 errors**. Only **3 were dead
links** — the other 5 are live
URLs the checker cannot reach, and their fix lands upstream (see below).
Every replacement here was
verified against the live target, not inferred from the URL shape.

## The three dead links

**`docs/adr/0002-…md`** — GitHub retired the
`repositories/configuring-branches-and-merges…/managing-protected-branches/`
path for the required-status-checks troubleshooting page. It now lives
under
`pull-requests/how-tos/merge-and-close-pull-requests/`. Verified 200
with **no redirect**, H1 reads
"Troubleshooting required status checks", and it still covers the
check-never-reports case the ADR
cites it for ("Associated checks stay in a 'Pending' state and block
merging").

An independent fresh-context lookup caught that my first replacement
here was itself a 301 — the
`collaborating-with-pull-requests/…` path redirects to the `how-tos/`
one. Both resolve today, but a
redirect is a second thing that can be retired, and lychee already hints
to prefer resolved URLs, so
the second commit swaps in the canonical target. Verified both
directions: the old path returns 301
with that Location, the new one returns 200 with none.

**`plugins/dometrain/README.md`** — Dometrain moved its plans page from
`/pro/` to `/dometrain-pro/`.
Verified 200, `<title>Dometrain Plans - Dometrain</title>`. The link
text stays "Dometrain Pro"
because the slug and the product name both still are.

**`plugins/source-control/skills/babysit-prs/reference/freshness.md`** —
the most interesting of the
three. `graphql/reference/enums` did not 404; it became a **navigation
index** and no longer carries
any enum definitions at all, which is why the failure was `Cannot find
fragment` rather than a dead
page. GitHub split the GraphQL reference by domain, so
`MergeStateStatus` now lives on the `pulls`
page. The replacement was verified structurally, not just by status
code: `id="enum-mergestatestatus"`
is present in the **served HTML** (so lychee's fragment check resolves
it, rather than the anchor
being JS-injected), and the page carries both descriptions this doc
quotes verbatim — "The head ref
is out of date" and "The merge is blocked".

## The other five are not content defects, and are fixed upstream

`lychee.toml` is a **`managed` component** for this repo per
`standards/distribution/sync-manifest.yml`,
so editing it here would be silently overwritten by the next sync. The
config half of this report is
therefore **melodic-software/standards#303**:

- **`www.gnu.org/software/coreutils/…` (429)** — verified 200. A 429 is
the server rate-limiting the
checker and lands on whichever host the shared runner IP is throttled
against that run, so the fix
is `accept`-ing 429 rather than excluding a healthy host that would just
be replaced by a
  different one next run.
- **`dl.acm.org` and `queue.acm.org` (403)** — 403 even with a full
browser User-Agent; no header
  tuning reaches them.
- **`docs.genius.com` (403)** — 200 with a browser User-Agent; the
documented bot-block case.
- **`www.ntia.gov` (SSL not trusted)** — the chain verifies locally
(`openssl s_client` →
`Verify return code: 0 (ok)`, curl 200 under strict verification). A
trust store failing an ECC
  chain, not an untrustworthy host.

This PR merging alone will not clear the report; #303 has to land and
sync. Flagging that plainly
rather than letting a half-clear look like a regression.

## Verification

Run with the real `lychee.toml` plus the proposed upstream config, over
all seven files the report
named:

```text
🔍 118 Total  🔗 116 Unique  ✅ 114 OK  🚫 0 Errors  👻 4 Excluded
```

All 8 reported errors resolved. Also run against this repo's gates:

- `scripts/check-changed-skills.sh origin/main` — 1 skill checked, 0
failed
- `scripts/check-contract-slice-prune.sh --check-diff origin/main` —
pass (no `docs/topics/` path is
touched; that file's two ACM URLs are handled by exclusion, not by
editing it)
- `scripts/check-changelog-parity.sh --check-bump origin/main` — pass.
**No plugin version bump**: a
corrected external URL in a reference doc changes no behavior contract,
no gate requires one, and
bumping `source-control` would collide with the in-flight bump on #1782
— the collision class
  tracked as #1746.
- `markdownlint-cli2`, `typos` — clean

## Related

- Fixes #640
- melodic-software/standards#303 — the upstream half; owns `lychee.toml`
for this repo
- #1746 — the concurrent version-bump collision class, the reason this
change deliberately bumps
  nothing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<https://claude.ai/code/session_01RhS3T7ShwJgKTrvk2Mvd3C>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The exclusion assertions only inspect `--dump` output, so nothing exercised
`accept`'s effect on a real response and the 429 behavior could have regressed
silently.

A dependency-free Python responder binds an ephemeral loopback port and answers
429 on one path and 404 on a control path. The real lychee binary runs against
scratch markdown for each, using the repository's own `lychee.toml`: the 429 case
must exit 0 and report an OK link, the 404 control must exit non-zero and name
the rejected status. The config excludes loopback, so `--include` re-admits the
fixture URL and a `--dump` assertion proves it is genuinely checked rather than
passing vacuously through the exclusion.

Mutation-verified: with `"429"` removed from `accept` in `lychee.toml`, the suite
fails with `accepted 429 exits 0 — exit expected 0 got 2` and the OK-count
assertion; restoring it returns the suite to green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STASxNiqEm7doy2Z65un4H
@kyle-sexton
kyle-sexton merged commit f73381c into main Jul 30, 2026
42 checks passed
@kyle-sexton
kyle-sexton deleted the fix/lychee-accept-429-and-bot-blocked-hosts branch July 30, 2026 21:23
@kyle-sexton
kyle-sexton restored the fix/lychee-accept-429-and-bot-blocked-hosts branch August 1, 2026 01:55
@kyle-sexton
kyle-sexton deleted the fix/lychee-accept-429-and-bot-blocked-hosts branch August 8, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant