Skip to content

ci(docs): stop failing the weekly link pass on a transient upstream status - #199

Merged
anilcancakir merged 3 commits into
masterfrom
ci/lychee-transient-status
Sep 7, 2026
Merged

anilcancakir merged 3 commits into
masterfrom
ci/lychee-transient-status

Conversation

@anilcancakir

@anilcancakir anilcancakir commented Sep 7, 2026

Copy link
Copy Markdown
Member

What

The weekly External Links pass no longer fails on a 429 or a 5xx, and it now keeps a one-day request cache.

--cache
--max-cache-age 1d
--accept '100..=103,200..=299,429,500..=504'

Plus an actions/cache restore step for .lycheecache, pinned by SHA like every other action here.

Why

Run 34130370451 failed with three 504 Gateway Timeouts from github.com, all of them this repo's own pages:

[504] https://github.com/fluttersdk/wind/compare/1.5.0...HEAD  (CHANGELOG.md 9:4)
[504] https://github.com/fluttersdk/wind/releases/tag/1.3.0    (CHANGELOG.md 95:4)
[504] https://github.com/fluttersdk/wind/releases/tag/1.4.1    (CHANGELOG.md 35:4)

All three load fine in a browser, and neither obvious explanation survives a look at the source:

  • Not a missing token. lychee-action's action.yml defaults its token input to ${{ github.token }} and entrypoint.sh exports it as GITHUB_TOKEN, so lychee is already authenticated. A token addresses rate limiting anyway, which arrives as 403 or 429, not 504.
  • Not a missing retry. lychee's --max-retries already defaults to 3, with a 1 second wait.

So the job was failing on the one thing it does not exist to catch. It hunts link rot, and a 5xx says the host is up and unhappy rather than that the link is gone. 404 and 410 still fail the job, which is what rot actually looks like.

The cache is the second half: a same-day re-run reuses the answers instead of asking every host again. The one-day age is deliberately shorter than the weekly cadence, so each Monday pass still checks every URL for real and the cache only helps the manual retry after a transient failure.

Testing

  • actionlint .github/workflows/docs-link-check.yml: clean
  • zizmor .github/workflows/docs-link-check.yml: no findings
  • Workflow dispatched on this branch to exercise the real external pass, not just the syntax: run 34150081246

Not included

--accept-timeouts stays off. A client-side timeout is not what failed here, and swallowing one would hide a host that hangs rather than answers. It is the next knob if a timeout ever shows up.

Summary by CodeRabbit

  • Chores
    • Improved external documentation link checks with temporary caching for faster retries.
    • Scheduled checks now revalidate links weekly.
    • Rate-limit and temporary server errors no longer cause checks to fail; other link failures remain reported.

…tatus

The 2026-09-07 scheduled run failed with three 504 Gateway Timeouts from
github.com, all of them this repo's own pages: the `compare/1.5.0...HEAD` link
and the 1.3.0 and 1.4.1 tag links, every one of which loads fine in a browser.

Neither of the two obvious explanations survives a look at the source. It is
not a missing token: `lychee-action`'s `action.yml` defaults its `token` input
to `${{ github.token }}` and `entrypoint.sh` exports it as `GITHUB_TOKEN`, so
lychee is already authenticated, and a token addresses rate limiting anyway,
which arrives as 403 or 429. It is not a missing retry either: lychee's
`--max-retries` defaults to 3.

So the job was failing on the one thing it does not exist to catch. It hunts
link ROT, and a 5xx says the host is up and unhappy rather than that the link
is gone. `--accept` now widens the default `100..=103,200..=299` with 429 and
`500..=504`; 404 and 410 still fail the job, which is what rot looks like.

The cache is the second half. A same-day re-run reuses the answers instead of
asking every host again, and the 1 day age is deliberately shorter than the
weekly cadence, so each Monday pass still checks every URL for real.

Verified: `actionlint` clean, `zizmor` clean, and the workflow dispatched on
this branch.
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 8a377499-3e12-4ee4-9e79-9548bc6cb87f

📥 Commits

Reviewing files that changed from the base of the PR and between 9794947 and c296a51.

📒 Files selected for processing (2)
  • .github/workflows/docs-link-check.yml
  • CHANGELOG.md

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 249cd96b-7e0d-4640-acc0-80434afe79d9

📥 Commits

Reviewing files that changed from the base of the PR and between 92f6d89 and 9794947.

📒 Files selected for processing (1)
  • .github/workflows/docs-link-check.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The documentation link-check workflow now restores a one-day Lychee cache keyed by commit SHA. Lychee uses the cache and accepts HTTP 429 and 500–504 responses while continuing to fail on missing-link statuses such as 404 and 410.

Changes

Docs link checking

Layer / File(s) Summary
Lychee cache and HTTP response policy
.github/workflows/docs-link-check.yml
The workflow restores and uses a one-day commit-keyed Lychee cache. The check accepts HTTP 429 and 500–504 responses but continues to fail on missing-link statuses such as 404 and 410.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 97949

The weekly external-link check will tolerate transient upstream rate-limit and server errors while retaining failures for missing links. The caching and response-policy change is ready to merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing the weekly documentation link check from failing on transient upstream HTTP statuses.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/lychee-transient-status

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@anilcancakir

Copy link
Copy Markdown
Member Author

Proved the flag rather than trusting it. The dispatched run passes (939 links, 935 OK, 4 excluded, 0 errors), but GitHub answered every request that time, so that run only shows the workflow is well-formed. So I pulled the exact binary CI installs, lychee 0.24.2, and drove it against controlled statuses from httpbingo.org.

Defaults, which is what run 34130370451 hit:

[404] https://httpbingo.org/status/404 | Rejected status code: 404 Not Found
[429] https://httpbingo.org/status/429 | Rejected status code: 429 Too Many Requests
[504] https://httpbingo.org/status/504 | Rejected status code: 504 Gateway Timeout
🔍 4 Total  ✅ 1 OK  🚫 3 Errors

With the accept list this PR adds:

[404] https://httpbingo.org/status/404 | Rejected status code: 404 Not Found
🔍 4 Total  ✅ 3 OK  🚫 1 Error     (exit 2)

So the 504 and the 429 stop failing the job and the 404 still fails it, which is the whole point: rot is still caught.

The cache half checks out too. A first --cache --max-cache-age 1d run writes .lycheecache and the second run over the same input answers from it, 568ms down to 200ms with the same 2 OK.

@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kodizm

kodizm Bot commented Sep 7, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The diagnosis is right and the flags parse and behave as claimed, but the cache does not save on the run it was added for, and the CHANGELOG entry this repo gives workflow-only changes is missing.

Major

.github/workflows/docs-link-check.yml:81actions/cache declares post-if: "success()", so .lycheecache is written only when the job passes. The step comment says the cache exists so "the cache only helps the manual retry after a transient failure" - but with --accept now swallowing 429 and 5xx, the runs that still fail are the ones that would be retried (a real 404, a client-side timeout, a connection error), and on those the save step is skipped and the retry re-asks every host. Split into actions/cache/restore + actions/cache/save with if: always() if the retry is meant to benefit. Evidence: curl .../actions/cache/55cc834.../action.ymlpost-if: "success()"; the same SHA does resolve to tag v6.1.0, so the pin itself is correct. (correctness)

CHANGELOG.md — no entry. CLAUDE.md puts "CI infra" under ### Quality for every behaviour-changing change, and the four most recent workflow-only changes in this repo each took one (CHANGELOG.md:51, :53, :55, :57 - all .github/workflows/** and nothing else). This change alters when a required-adjacent job goes red, which is squarely behaviour.

Minor

.github/workflows/docs-link-check.yml:105--max-cache-age 1d is already the default in the pinned lychee 0.24.2 (--max-cache-age ... [default: 1d]), so the flag is a no-op. Harmless, but the comment reads as if it is overriding something longer; either drop the flag or say it is pinning the default against a future change.

.github/workflows/docs-link-check.yml:106--cache-exclude-status is left unset, so an accepted 429 or 504 is now written to .lycheecache as a normal result and replayed for the rest of the day. That means a same-day re-run cannot re-probe the host that just failed transiently - it reads the transient status back out of the cache. Bounded by the weekly cadence, and possibly what you want, but --cache-exclude-status '429,500..=599' is the knob if the retry should really re-ask.

.gitignore.lycheecache is not ignored. --cache writes it into the repo root, so anyone reproducing the external pass locally now leaves an untracked file behind (mine did).

Tests

No test surface for a workflow file. Verified instead by running the pinned lychee binary with the exact flag string from the diff.

Checks I ran

  • Downloaded lychee 0.24.2 (the version lychee-action v2.9.0 pins) and ran the PR's exact flags against doc/getting-started/*.md: 13 Total ... 13 OK 0 Errors, exit 0, and .lycheecache written in the working directory - so the --accept range parses and the cache path matches the actions/cache path:.
  • lychee --help on that binary: --accept [default: 100..=103,200..=299], --max-cache-age [default: 1d], --cache-exclude-status with no default.
  • Read lychee-action entrypoint.sh at the pinned SHA: it ends in eval lychee ... ${ARGS}, so the single quotes around the accept list are shell-processed rather than passed through literally. No quoting bug.
  • Resolved actions/cache tag v6.1.0 via the GitHub API: SHA matches the pin.
  • actionlint and zizmor are not installed in this environment, so I did not reproduce those two results from the description.
  • One file changed, fully read; nothing was listed as changed_without_diff or dropped_for_size.

Two review findings, both verified against the source before acting.

The cache does not save on the run it was added for. `actions/cache` declares
`post-if: "success()"` at the pinned SHA, so the save step is skipped whenever
the job fails, which after this change is exactly the run a retry would follow:
a 429 or a 5xx no longer fails, so a red run means a real 404. Splitting into
`restore` plus `save` with `if: always()` would fix that, but the whole external
pass costs under four seconds for 939 links (run 34150081246), so there is
nothing worth buying back. Dropping it also retires three smaller notes in one
move: `--max-cache-age 1d` was restating lychee's own default, an accepted 429
or 504 would have been replayed out of the cache for the rest of the day, and
`.lycheecache` would have needed a `.gitignore` line for anyone reproducing the
pass locally.

The CHANGELOG entry was missing. `CLAUDE.md` files CI infrastructure under
`### Quality`, and 1.4.1 carries four workflow-only entries there. This change
alters when a job goes red, which is behaviour.

Verified again after the edit: `actionlint` clean, `zizmor` clean,
`tool/check-docs.py` 0 issues.
@anilcancakir

Copy link
Copy Markdown
Member Author

Both findings verified against the source and taken in caff7e3, and the cache is gone rather than repaired.

actions/cache does declare post-if: "success()" at the pinned SHA, so the save is skipped on exactly the runs a retry would follow: after this change a 429 or a 5xx no longer fails the job, so a red run means a real 404. The restore plus save split with if: always() would fix that, but the whole external pass costs under four seconds for 939 links (run 34150081246), so there is nothing worth buying back. Dropping the cache retires the two minors and the .gitignore note in the same move: --max-cache-age 1d was restating lychee's own default, an accepted 429 or 504 would have been replayed out of the cache for the rest of the day, and .lycheecache would have needed ignoring for anyone reproducing the pass locally.

The CHANGELOG entry is in under ### Quality, which is where CLAUDE.md files CI infrastructure and where 1.4.1's four workflow-only entries live. It carries the measurement rather than the intent: with lychee 0.24.2's defaults a 404, a 429 and a 504 all fail the run, and with the accept list the 429 and the 504 pass while the 404 still fails it with exit 2.

What is left in the diff is one flag and its comment. Re-verified after the edit: actionlint clean, zizmor clean, tool/check-docs.py 0 issues, and the workflow dispatched again on this branch.

@kodizm

kodizm Bot commented Sep 7, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

Incremental re-review of caff7e3: both earlier findings are settled, the diff is now one flag plus its CHANGELOG entry, and it looks correct.

Since my last pass: the actions/cache step is gone (which retires the post-if: success() finding and all three minors that hung off it - the redundant --max-cache-age 1d, the unset --cache-exclude-status, and the missing .gitignore entry), and the ### Quality CHANGELOG entry landed in the right section for the right release.

Minor

.github/workflows/docs-link-check.yml:82 — the comment says --accept widens the default "with 429 and the 5xx range", but the flag is 500..=504, not 500..=599. So a Cloudflare 520/521/522/530 or a 508 still fails the pass, which is the same class of transient the change exists to stop failing on. Either widen the range or say "500-504" in the comment; the code is defensible as-is, the sentence describing it is not. (maintainability)

Tests

No test surface for a workflow file. I re-parsed the YAML and reproduced the docs gate instead; the accept-list behaviour was already measured against the pinned lychee 0.24.2 in the thread above and I am not re-litigating it.

Checks I ran

  • python3 YAML parse of docs-link-check.yml: parses, external job now has 2 steps, and the assembled args are exactly --no-progress --accept '100..=103,200..=299,429,500..=504' 'doc/**/*.md' '*.md' 'llms.txt' with fail: true - so removing the cache step left the arg block well-formed.
  • python3 tool/check-docs.py: checked 72 doc pages + 3 linking files against 171 demo routes: 0 issue(s).
  • grep -rn "lycheecache\|actions/cache" across the repo: only the explanatory comment at docs-link-check.yml:89 and the CHANGELOG line - no orphaned path, key or ignore rule left behind.
  • CHANGELOG placement: ### Quality sits after ### Fixed under [Unreleased], matching 1.4.1, 1.4.0, 1.3.0 and 1.2.0.
  • actionlint and zizmor are still not installed here, so those two results from the description remain unreproduced by me.
  • Two files changed, both fully read; nothing was listed as changed_without_diff or dropped_for_size.

The review caught the comment claiming "the 5xx range" over a flag that read
500..=504, and the code is what was wrong. Stopping at 504 covers the failure
that prompted this change and leaves the same class failing one door down:
Cloudflare answers 520, 521, 524 and 530 for an origin that did not respond,
and 508 sits outside the range too.

Measured on the pinned lychee 0.24.2 over 503, 504, 508, 521, 524, 530 and a
404. The narrow range rejects five of the seven, the widened one rejects the
404 alone, which is the only link in the list that is actually gone.

`actionlint` clean, `zizmor` clean, `tool/check-docs.py` 0 issues.
@anilcancakir

Copy link
Copy Markdown
Member Author

Right, and the code was the wrong half. Fixed in c296a51 by widening the range rather than narrowing the sentence: stopping at 504 covers the failure that prompted this and leaves the same class failing one door down, since Cloudflare answers 520, 521, 524 and 530 for an origin that did not respond, and 508 sits outside it too.

Measured on the pinned lychee 0.24.2 over 503, 504, 508, 521, 524, 530 and a 404:

--accept '...,429,500..=504'   ->  🔍 7 Total  ✅ 2 OK  🚫 5 Errors   (404, 508, 521, 524, 530)
--accept '...,429,500..=599'   ->  🔍 7 Total  ✅ 6 OK  🚫 1 Error    (404)

The 404 is the only link in that list that is actually gone, and it is the only one that still fails the job. The comment and the CHANGELOG entry both carry the reason for the wider bound now.

actionlint clean, zizmor clean, tool/check-docs.py 0 issues, and the workflow dispatched again on this branch.

@kodizm

kodizm Bot commented Sep 7, 2026

Copy link
Copy Markdown

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

Incremental re-review of c296a51: the accept range now matches the reasoning behind it, and I have nothing left to flag.

Since caff7e3: --accept widened from 500..=504 to 500..=599, and the workflow comment plus the ### Quality CHANGELOG entry both carry the reason (Cloudflare's 520/521/524/530 being the same "origin did not answer" class one door down). That settles the maintainability minor from my last pass - fixing the code rather than the sentence was the right half. Nothing else changed.

Worth stating plainly since it is the whole risk surface of the widening: 500..=599 means the pass no longer distinguishes a permanently broken host from a briefly unhappy one, and 501/505 now pass too. That is the intended trade - a 5xx is a host answering, not a link that is gone - and 404/410 still fail the job, which is what rot looks like. Connection errors, DNS failures and client-side timeouts are not statuses and are unaffected, so they still fail.

Tests

No test surface for a workflow file. The accept-list behaviour was measured against the pinned lychee 0.24.2 in the thread above, including the 503/508/521/524/530 probe; I am not re-litigating it.

Checks I ran

  • Read the assembled arg block at head: --no-progress --accept '100..=103,200..=299,429,500..=599' 'doc/**/*.md' '*.md' 'llms.txt' with fail: true, and the external job is still two steps (checkout + lychee) with no cache leftovers.
  • python3 tool/check-docs.py: checked 72 doc pages + 3 linking files against 171 demo routes: 0 issue(s).
  • git log --oneline -3: confirms c296a51 is the only commit since my last review.
  • lychee, actionlint and zizmor are not installed in this environment, so the three results in the latest comment remain unreproduced by me; PyYAML is also absent here, so I read the YAML rather than parsing it this pass.
  • Two files changed, both fully read; nothing was listed as changed_without_diff or dropped_for_size.

@anilcancakir
anilcancakir merged commit 5911a74 into master Sep 7, 2026
17 checks passed
@anilcancakir
anilcancakir deleted the ci/lychee-transient-status branch September 7, 2026 20:41
@kodizm kodizm Bot mentioned this pull request Sep 7, 2026
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