fix(ci): retry flaky documentation link checks - #12746
Conversation
Signed-off-by: xianlubird <xianlubird@gmail.com>
|
/ok to test ee60c9d |
WalkthroughThe documentation workflow now captures the first lychee check result without failing immediately. If the check fails, it removes unsuccessful cache entries and runs a second check that can fail the job. ChangesDocumentation link validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/docs-link-check.yml (1)
59-60: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake cache cleanup status-aware.
Line 59 matches any
,2xx,substring in the full cache line. A URL containing,200,can preserve an entry whose actual status is an error, so the retry can reuse stale data. Lychee stores URL/status/timestamp records and does not normally persistCacheStatus::Errorentries. (lychee.cli.rs)Remove this shell rewrite, or parse the status field directly. Preserve the statuses that
--accepttreats as valid.Suggested fix
- grep -E ',2[0-9]{2},' .lycheecache > .lycheecache.tmp || true + awk -F',' '$(NF-1) ~ /^(2[0-9][0-9]|403|429)$/ { print }' \ + .lycheecache > .lycheecache.tmp🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/docs-link-check.yml around lines 59 - 60, Remove the grep/mv cache rewrite in the docs link-check workflow, or replace it with parsing that validates the cache record’s actual status field rather than matching any URL substring. Preserve only entries whose status is accepted by the workflow’s --accept configuration, ensuring error-status records cannot be reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/docs-link-check.yml:
- Around line 59-60: Remove the grep/mv cache rewrite in the docs link-check
workflow, or replace it with parsing that validates the cache record’s actual
status field rather than matching any URL substring. Preserve only entries whose
status is accepted by the workflow’s --accept configuration, ensuring
error-status records cannot be reused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a4baf2ec-52b3-496c-8bd5-36b4cbf31f71
📒 Files selected for processing (1)
.github/workflows/docs-link-check.yml
This comment has been minimized.
This comment has been minimized.
|
Looks like the SGLang failure is unrelated to this PR—the tests all passed, but the self-hosted runner hit a container error afterward. The new lychee retry worked as expected: the first check timed out on an external link, then the retry passed. |
Signed-off-by: xianlubird <xianlubird@gmail.com>
|
/ok to test cb319b0 |
|
The |
|
/ok to test 7dc91f4 |
Summary
.lycheecachebefore retrying so the second attempt performs real network requests instead of replaying cached errors.Background
PR #12741 exposed two independent transient failures in the repository-wide documentation link check. The first run reported HTTP 500/502 responses for existing GitHub pull request links. A new commit retriggered the workflow, but the next run failed on a different existing URL—the Kubernetes Ingress documentation—with a connection error. That URL returned HTTP 200 immediately when checked again.
The workflow already configures lychee with four request retries and a 30-second request timeout. However, lychee v0.24.2 does not classify ordinary connection errors as retryable, so
--max-retriesdoes not protect this case. Request-level retries can also be exhausted when an external host remains temporarily unhealthy.Implementation
The first lychee action records its real exit code without ending the job immediately. When that code is nonzero, the workflow parses the cache status field, keeps only statuses accepted by this workflow (
2xx,403, and429), and runs the same complete lychee command again. Both attempts reference the same YAML-anchored argument block. The retry uses the action's normalfail: truebehavior, so persistent broken links remain blocking.This does not add any URL to
.lycheeignore, accept connection errors, or usecontinue-on-errorto make the job green.Validation
pre-commit run check-yaml --files .github/workflows/docs-link-check.ymlactionlint v1.7.12 .github/workflows/docs-link-check.yml2xx,403,429,404, and comma-containing URL recordsgit diff --checkSummary by CodeRabbit