Migrate from markdown-link-check to lychee #2219
Conversation
The previous setup using Wandalen/wretry.action wrapper with tcort/markdown-link-check was silently failing due to a Node.js 21 ESM compatibility bug. The markdown-link-check tool crashed on every file, but the crash message didn't match the error detection pattern, so CI falsely reported "All links are good!" This migration to lychee provides: - Built-in retry for ALL transient errors (502, 503, 504), not just 429 - Proper exit codes (2 = link failures) - Faster execution (Rust-based, async) - Result caching for faster repeated runs - No Docker/Node.js compatibility issues Fixes #2217 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughReplaces the markdown-link-check pipeline with Lychee: deletes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Cache as actions/cache@v4
participant Lychee as lycheeverse/lychee-action@v2
participant Repo as Repository (.lychee.toml + docs)
GH->>Cache: restore `.lycheecache` (key: lychee-...)
GH->>Lychee: run lychee with `--config .lychee.toml` on targets (docs/, **/*.md)
Lychee->>Repo: read `.lychee.toml` (accept, timeout, retries, excludes, cache)
Lychee->>Repo: scan markdown files and request external URLs
Lychee-->>GH: return results (pass/fail)
GH->>Cache: save updated `.lycheecache`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
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. Comment |
Code Review: Migrate from markdown-link-check to lycheeOverall Assessment✅ Excellent PR - This is a well-researched, necessary fix that addresses a critical CI failure. The migration from markdown-link-check to lychee is well-justified and properly implemented. Strengths
Configuration ReviewGood Choices
Potential Improvements
Testing ConcernsExpected CI FailureThe PR description states this will fail CI because it correctly detects ~26 broken links. Recommendation: Before merging, you should either:
Option 1 is cleanest from a CI health perspective (aligns with CLAUDE.md's "Master Branch Health Monitoring" guidelines). Verification Steps NeededAccording to CLAUDE.md, you should:
Security & Best PracticesSecurity
Pin Action VersionConsider pinning the action to a specific version instead of @v2 for better security and reproducibility. Performance Considerations
Optimization Ideas:
Alignment with CLAUDE.md GuidelinesPost-Merge RequirementsAfter merging:
Final RecommendationsBefore Merging:
After Merging:
VerdictLGTM with minor suggestions 👍 This is a necessary and well-executed migration. The suggestions above are optimizations and safeguards, not blockers. The core change is solid. Estimated risk: Low - Switching to a more reliable tool with proper testing Great work diagnosing and fixing this! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/check-markdown-links.yml (1)
3-15:paths: '**.md'but lychee only scansdocs/— confirm intended coverage. If you want repo-wide markdown link checking, adjustargs(or narrow the workflowpathstodocs/**.md).- args: --config .lychee.toml docs/ + args: --config .lychee.toml .
🧹 Nitpick comments (1)
.github/workflows/check-markdown-links.yml (1)
26-37: Cache key is effectively “no cache” across commits; use a stable key.${{ github.sha }}guarantees a new primary key every commit, reducing hit rate and increasing cache churn. Prefer OS + config hash (and optionally a docs hash) so caches actually get reused.- name: Restore lychee cache uses: actions/cache@v4 with: path: .lycheecache - key: cache-lychee-${{ github.sha }} - restore-keys: cache-lychee- + key: lychee-${{ runner.os }}-${{ hashFiles('.lychee.toml') }} + restore-keys: | + lychee-${{ runner.os }}-Also, per workflow guidelines, please ensure you’ve tested the updated workflow locally (e.g., with
actor an equivalent) before iterating via CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/markdown-link-check-config.json(0 hunks).github/workflows/check-markdown-links.yml(2 hunks).gitignore(1 hunks).lychee.toml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/markdown-link-check-config.json
🧰 Additional context used
📓 Path-based instructions (1)
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Changes to CI workflows require comprehensive local testing BEFORE pushing to avoid CI failure cycles
Files:
.github/workflows/check-markdown-links.yml
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements using format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)`
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to **/.github/workflows/*.yml : Changes to CI workflows require comprehensive local testing BEFORE pushing to avoid CI failure cycles
Applied to files:
.github/workflows/check-markdown-links.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
- GitHub Check: examples (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: precompile-check
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: claude-review
🔇 Additional comments (2)
.gitignore (1)
92-94:.lycheecacheignore rule looks correct..lychee.toml (1)
4-49: No issues identified. All configuration keys and types in.lychee.toml(accept, max_retries, retry_wait_time, timeout, exclude, exclude_path, include_fragments, cache, max_cache_age) are valid for lychee v0.21.0 (the default version used by lycheeverse/lychee-action@v2). The schema is correct and CI will not silently ignore these settings.
Removed exclusions that were incorrectly added: - blog.shakacode.com - returns 200 (redirects to Medium) - reactrails.com - returns 200 - react-webpack-rails-tutorial - not archived, broken links are real These sites work fine and the tutorial repo links are genuinely broken (the specific files no longer exist at those paths). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
react.dev returns 308 for some URLs - this is a valid permanent redirect, not a broken link. Added 308 to accept list alongside 200 and 206. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review - PR #2219: Migrate from markdown-link-check to lychee✅ Overall AssessmentExcellent migration! This PR solves a critical CI reliability issue with a well-researched, properly configured replacement. The solution is cleaner, faster, and more robust than the previous setup. 🎯 Strengths1. Problem Clearly Identified
2. Configuration Migration is Accurate✅ All exclusion patterns properly migrated from 3. Significant Performance Improvement
4. Proper CI Integration✅ Cache key strategy is correct with fallback 🔍 Minor Observations & Suggestions1. Configuration: Consider Adding
|
Code Review: Migration to Lychee Link CheckerI've reviewed this PR and it looks excellent overall! This is a well-executed migration that addresses a critical CI failure. Here's my detailed feedback: ✅ Strengths
🔍 Potential Issues1. Limited Scope - Only Checking
|
ihabadham
requested review from
alexeyr-ci2 and
justin808
and removed request for
justin808
.lychee.toml
Outdated
|
|
||
| # Accept successful responses and permanent redirects | ||
| # 200 = OK, 206 = Partial Content, 308 = Permanent Redirect | ||
| accept = ["200", "206", "308"] |
There was a problem hiding this comment.
It was in the old config. Not sure why I added it then but we don't actually need it. Removed
|
@ihabadham can you have CC post a reply here if you want to do the suggestions in #2219 (comment) in this PR or make a follow up or not do it. |
Code Review: Migrate from markdown-link-check to lychee✅ Overall AssessmentThis is a well-executed migration that solves a critical problem with the existing link checker. The PR is well-documented, addresses a real issue (#2217), and uses a modern, reliable tool. 🎯 Strengths
🔍 Suggestions & Questions1. Accept Status CodesWhy exclude 301 (Permanent Redirect)? The old config had aliveStatusCodes: [200, 206]. You added 308, but consider if 301 should also be accepted. Many sites use 301 for permanent redirects. 2. Retry ConfigurationThe old config used exponential backoff (fallbackRetryDelay: 30s with retryCount: 5). Consider if lychee supports exponential backoff or if 10 seconds is sufficient. For transient 502/503 errors, servers may need more time. 3. Missing Headers ConfigurationThe old config had httpHeaders for docs.github.com and github.com with Accept: text/html. Does lychee need this? GitHub sometimes requires proper Accept headers. Check if lychee handles this automatically. 4. Cache Key StrategyUsing github.sha means each commit gets a new cache, but restore-keys will restore from previous runs. This is good for CI efficiency. ✅ Optional enhancement: Consider adding branch to the cache key for PR-specific caching. 5. Testing Before MergeCritical: Since this PR will correctly detect ~26 broken links:
This prevents blocking unrelated PRs while broken links are being fixed. 🔒 Security Considerations✅ No security concerns
⚡ Performance Considerations✅ Excellent improvement
📝 Minor Nits
✅ Final VerdictLGTM with minor suggestions! The migration is well-thought-out and solves a real problem. The PR is ready to merge once you:
Great work! This will significantly improve the reliability of link checking in CI. 🤖 Generated with Claude Code |
206 (Partial Content) was inherited from the old markdown-link-check config but is unnecessary - no URLs in our docs return 206. Lychee doesn't send Range headers, so we'd never trigger a 206 response. Keeping the accept list minimal: 200 (OK) and 308 (Permanent Redirect). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Expand scope to check root *.md files (README, CHANGELOG, etc.) in addition to docs/ directory - Increase retry resilience: 5 retries with 30s wait (was 3/10s) to match previous markdown-link-check behavior - Document why medium.com is excluded (returns 403 for bots) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Config improvements: - Expand scope to include react_on_rails_pro/docs/ and react_on_rails_pro/*.md - Add path exclusions for node_modules and spec directories - Add docs.google.com to URL exclusions (private docs require auth) - Adjust retry config to 4 retries with 15s wait (balanced approach) Fixed broken links in react_on_rails_pro/: - CONTRIBUTING.md: Update helper.ts path after monorepo restructure - CHANGELOG.md: Fix shakacode.com RSC tutorial URL - docs/release-notes/4.0.md: Fix RSC tutorial relative links - docs/release-notes/v4-react-server-components.md: Fix RSC tutorial link 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Response to Claude Code Review SuggestionsWe investigated each suggestion and addressed them as follows: ✅ Addressed
❌ Not Needed (Investigated)
Additional Changes Made
All changes are in commits:
|
Code Review - PR #2219: Migrate from markdown-link-check to lychee✅ Overall AssessmentThis is an excellent migration that solves a critical CI reliability issue. The PR correctly addresses the silent failure problem described in #2217 and switches to a modern, faster, and more reliable link checker. 🎯 Strengths1. Proper Root Cause Analysis
2. Smart Tool ChoiceLychee is a significantly better choice:
3. Clean Configuration MigrationThe
4. Workflow Improvements
🔍 Code Quality ReviewWorkflow File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/check-markdown-links.yml (1)
26-31: Optimize cache key strategy for better hit rates.The cache key
cache-lychee-${{ github.sha }}creates a unique cache entry per commit, which means cache hits will only occur if the same commit runs twice. This defeats the primary benefit of caching. Consider using a key that persists across commits unless the configuration changes:- key: cache-lychee-${{ github.sha }} + key: cache-lychee-${{ hashFiles('.lychee.toml') }}This approach:
- Creates a new cache entry only when
.lychee.tomlchanges- Reuses the same cache across commits within the same configuration
- Significantly improves cache hit rates and CI performance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/check-markdown-links.yml(2 hunks).lychee.toml(1 hunks)react_on_rails_pro/CHANGELOG.md(1 hunks)react_on_rails_pro/CONTRIBUTING.md(11 hunks)react_on_rails_pro/docs/release-notes/4.0.md(1 hunks)react_on_rails_pro/docs/release-notes/v4-react-server-components.md(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- react_on_rails_pro/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .lychee.toml
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}: ALWAYS runbundle exec rubocopand fix ALL violations before every commit/push
ALWAYS ensure files end with a newline character before committing
Files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
**/*.{js,ts,jsx,tsx,json,css,scss,md}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS let Prettier handle ALL formatting - never manually format code
Files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
**/.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Changes to CI workflows require comprehensive local testing BEFORE pushing to avoid CI failure cycles
Files:
.github/workflows/check-markdown-links.yml
🧠 Learnings (30)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements using format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)`
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements using format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)`
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
react_on_rails_pro/docs/release-notes/4.0.mdreact_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to /CHANGELOG.md,/CHANGELOG_PRO.md : Do NOT add changelog entries for: linting, formatting, refactoring, tests, or documentation-only fixes
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to react_on_rails_pro/**/*.{js,ts,jsx,tsx,json,css,scss} : The `react_on_rails_pro/` directory has its own Prettier/ESLint configuration separate from the root; CI lints both directories separately
Applied to files:
react_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for Pro-only features, bug fixes, and improvements using the same format: `[PR number](URL) by [username](URL)`
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to **/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md} : ALWAYS run `bundle exec rubocop` and fix ALL violations before every commit/push
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to lib/react_on_rails/**/*.rb : Create corresponding RBS signature files in `sig/react_on_rails/` for new Ruby files and add them to Steepfile for type checking
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Monorepo contains separate open-source and Pro packages; changes affecting both require updating both `/CHANGELOG.md` and `/CHANGELOG_PRO.md`
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to lib/react_on_rails/**/*.rb : Engine code has limited access to host application; use `Rails.root` for host app paths and `ReactOnRails.configure` for engine configuration
Applied to files:
react_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to lib/generators/react_on_rails/**/*.rb : Generators run in host app context, not engine context; do not assume host app structure (e.g., `app/javascript/` may not exist in older apps)
Applied to files:
react_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to spec/dummy/**/*.{spec,test}.{js,ts} : Playwright E2E tests use `cypress-on-rails` gem for Rails integration; use helpers like `app('clean')`, `appEval()`, and `appFactories()` for test setup instead of UI automation
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
react_on_rails_pro/CONTRIBUTING.mdreact_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to **/.github/workflows/*.yml : Changes to CI workflows require comprehensive local testing BEFORE pushing to avoid CI failure cycles
Applied to files:
react_on_rails_pro/CONTRIBUTING.md.github/workflows/check-markdown-links.yml
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to spec/dummy/e2e/playwright/app_commands/**/*.rb : Create custom app commands for Playwright E2E tests in this directory using the `command` syntax provided by `cypress-on-rails` gem
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Prefer local testing over CI iteration to save time (15-minute rule: if 15 more minutes of local testing would prevent a CI failure, spend the time locally first)
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to package.json,pnpm-lock.yaml : Test build scripts after modifying package.json or dependencies: run `pnpm run prepack` and `pnpm run yalc.publish` to verify build pipeline
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to spec/dummy/**/*.{js,ts} : Install Playwright browsers via `cd spec/dummy && pnpm playwright install --with-deps` before running E2E tests
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-12-10T09:27:45.105Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.105Z
Learning: Applies to spec/dummy/config/webpack/**/*.js : When debugging Webpack configuration issues, create temporary debug scripts to inspect rules and loaders; clean them up before committing
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Applied to files:
react_on_rails_pro/CONTRIBUTING.md
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
react_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-06-09T07:58:02.646Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadGenerator.ts:124-168
Timestamp: 2025-06-09T07:58:02.646Z
Learning: In React Server Components (RSC) implementations, explicit error handling in RSC payload generation streams (like in RSCPayloadGenerator.ts) is not needed because errors will propagate to client components that fetch the RSC payloads (such as RSCRoute components). React's built-in error handling mechanisms and ErrorBoundary components are sufficient for handling these errors at the component level, which allows for better user experience with appropriate error UI fallbacks.
Applied to files:
react_on_rails_pro/docs/release-notes/v4-react-server-components.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
Applied to files:
react_on_rails_pro/docs/release-notes/v4-react-server-components.md
🪛 markdownlint-cli2 (0.18.1)
react_on_rails_pro/CONTRIBUTING.md
34-34: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
48-48: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: dummy-app-integration-tests (3.4, 22, latest)
- GitHub Check: examples (3.4, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: claude-review
- GitHub Check: markdown-link-check
🔇 Additional comments (4)
.github/workflows/check-markdown-links.yml (2)
9-9: Correct trigger paths for .lychee.toml.Adding
.lychee.tomlto workflow triggers is the right call—the workflow should re-run whenever the configuration changes.Also applies to: 14-14
33-36: Verify comprehensive local testing and path coverage before landing.The migration from markdown-link-check to lychee is sound. However, please confirm:
Local testing: Per workflow guidelines, comprehensive local testing should be performed before pushing. Have you verified the lychee setup works locally with the specified paths and
.lychee.tomlconfiguration?Path coverage: Verify that the specified paths (
docs/,*.md,react_on_rails_pro/docs/,react_on_rails_pro/*.md) cover all markdown files in the repository that may contain external links.Known broken links: The PR notes ~26 broken URLs will be detected by lychee (and should fail CI as expected). Confirm the team has a plan to fix these separately and that this is an acceptable interim state.
react_on_rails_pro/docs/release-notes/v4-react-server-components.md (1)
48-48: The relative link path is correct. The target file exists atreact_on_rails_pro/docs/react-server-components/tutorial.md, and the relative path../react-server-components/tutorial.mdproperly navigates from therelease-notes/directory. No action required.react_on_rails_pro/docs/release-notes/4.0.md (1)
14-14: > Likely an incorrect or invalid review comment.
| #### The seven rules of a great git commit message | ||
|
|
||
| > Keep in mind: This has all been said before. |
There was a problem hiding this comment.
Fix heading level increment (MD001): use ### instead of ####.
Line 34 jumps from ## Commit Messages to an H4. This trips markdownlint and makes the doc hierarchy uneven.
-#### The seven rules of a great git commit message
+### The seven rules of a great git commit message📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### The seven rules of a great git commit message | |
| > Keep in mind: This has all been said before. | |
| ### The seven rules of a great git commit message | |
| > Keep in mind: This has all been said before. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
34-34: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
In react_on_rails_pro/CONTRIBUTING.md around lines 34 to 36, the heading "The
seven rules of a great git commit message" is using an H4 (####) immediately
after the H2 "Commit Messages", which violates markdown heading hierarchy
(MD001); change that H4 to an H3 (###) so the document uses an incrementally
nested heading level and resolves the markdownlint error.
| When making doc changes, we want the change to work on both https://www.shakacode.com/react-on-rails-pro/docs/ and when browsing the GitHub repo. | ||
| The issue is that the Shakacode site is generated only from files in [`docs`](./docs), so any references from them to non-doc files must use the full GitHub URL. |
There was a problem hiding this comment.
Avoid bare URL (MD034): turn the site URL into a link.
Line 48 uses a bare URL; prefer a markdown link for consistency and lint compliance.
-When making doc changes, we want the change to work on both https://www.shakacode.com/react-on-rails-pro/docs/ and when browsing the GitHub repo.
+When making doc changes, we want the change to work on both [shakacode.com docs](https://www.shakacode.com/react-on-rails-pro/docs/) and when browsing the GitHub repo.🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
48-48: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In react_on_rails_pro/CONTRIBUTING.md around lines 48 to 49, replace the bare
URL "https://www.shakacode.com/react-on-rails-pro/docs/" with a markdown link to
avoid MD034; update the sentence to use the link text (e.g. [Shakacode React on
Rails Pro docs]) and the URL in parentheses so the site and repo rendering are
consistent and linter-compliant.
| > [!NOTE] > `yalc` makes the NPM package available globally on the machine. | ||
| > So, if you have the repo checked out more than once to compare behavior between branches, | ||
| > make sure to run `yarn install` every time you switch to a new copy. | ||
There was a problem hiding this comment.
Fix malformed NOTE admonition so it renders correctly.
Line 181 currently has > [!NOTE] > ..., which won’t render as an admonition (and may collapse into a single quoted line). Use a proper multi-line blockquote:
-> [!NOTE] > `yalc` makes the NPM package available globally on the machine.
-> So, if you have the repo checked out more than once to compare behavior between branches,
-> make sure to run `yarn install` every time you switch to a new copy.
+> [!NOTE]
+> `yalc` makes the NPM package available globally on the machine.
+> So, if you have the repo checked out more than once to compare behavior between branches,
+> make sure to run `yarn install` every time you switch to a new copy.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > [!NOTE] > `yalc` makes the NPM package available globally on the machine. | |
| > So, if you have the repo checked out more than once to compare behavior between branches, | |
| > make sure to run `yarn install` every time you switch to a new copy. | |
| > [!NOTE] | |
| > `yalc` makes the NPM package available globally on the machine. | |
| > So, if you have the repo checked out more than once to compare behavior between branches, | |
| > make sure to run `yarn install` every time you switch to a new copy. | |
🤖 Prompt for AI Agents
In react_on_rails_pro/CONTRIBUTING.md around lines 181 to 184, the NOTE
admonition is malformed as it currently reads "`> [!NOTE] > ...`"; remove the
extra `>` after `[!NOTE]` and convert the following lines into a proper
multi-line blockquote by making the first line just `> [!NOTE]` and prefixing
each subsequent line of the note with `> ` so the admonition renders correctly.
- Fix reactrails.com link in PROJECTS.md (http → https)
- Remove caseflow link from PROJECTS.md (404 - repo no longer public)
- Add URL exclusions for sites that block automated requests:
- blog.shakacode.com, foxford.ru, airgoat.com, first.io,
estately.com, hiring.careerbuilder.com
- Exclude SUMMARY.md from checking (GitBook TOC with old doc
structure links, intentionally kept for redirects)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review: Migrate from markdown-link-check to lychee✅ Overall AssessmentThis is an excellent migration that replaces a broken link-checking setup with a modern, reliable alternative. The changes are well-executed and address a critical CI issue where link checking was silently failing. 🎯 Strengths1. Problem Diagnosis & Solution Choice
2. Configuration Migration QualityThe
3. Workflow Improvements
4. Documentation Updates
🔍 Detailed ReviewConfiguration File (.lychee.toml)Good:
Minor suggestion:
Workflow ChangesGood:
Potential improvement:
Documentation ChangesExcellent:
🚨 Considerations1. Expected CI FailuresThe PR description correctly notes this will fail CI due to ~26 broken links. This is expected behavior and shows the tool is working correctly. The old setup was hiding these failures. Recommendation: Merge this PR once the link fixes from related PRs are applied, OR update .lychee.toml to temporarily exclude known-broken URLs with comments indicating they need fixing. 2. Exclusion Pattern CoverageThe exclusions look good, but verify these patterns are still needed:
3. Gitignore Addition✅ Good addition of 📋 Testing RecommendationsBefore merging:
🎨 Code Quality
📝 Minor Nitpicks
✅ Approval RecommendationAPPROVE with minor suggestions This PR:
The expected CI failures are not a blocker—they prove the tool is working correctly. 🚀 Next Steps
Great work on identifying and fixing this silent failure! 🎉 |
Returns 403 for automated requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Replaces the broken
markdown-link-checksetup withlycheefor link checking.Problem
The previous setup using
Wandalen/wretry.actionwrapper withtcort/markdown-link-checkwas silently failing due to a Node.js 21 ESM compatibility bug (see #2217). The tool crashed on every file, but the crash message didn't match the error detection pattern, so CI falsely reported "All links are good!" - while actually checking zero links.Solution
Migrate to lychee, a Rust-based link checker that:
Changes
.github/markdown-link-check-config.jsonwith.lychee.tomllycheeverse/lychee-action@v2.lycheecacheto.gitignoreTest Plan
Note
This PR will fail CI because it correctly detects ~26 broken links that the previous setup missed. Those links need to be fixed in a separate PR (like #2216).
Fixes #2217
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.