feat: add redirect loop checker #7251
Conversation
WalkthroughThe PR introduces a GitHub Actions workflow, Node.js script, and supporting documentation to detect redirect loops in a redirects file. A new npm script and workflow configuration are added to automatically check for redirect issues on PRs and pushes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Dangerous URL checkNo absolute URLs to prisma.io/docs found. |
Redirect checkThis PR probably requires the following redirects to be added to static/_redirects:
|
Deploying docs with
|
| Latest commit: |
4e00bba
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://afeabb4b.docs-51g.pages.dev |
| Branch Preview URL: | https://redirect-loop-checker.docs-51g.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/README.md (1)
44-50: Add language specifiers to fenced code blocks.The markdown code blocks at lines 44-50 and 53-63 are missing language specifiers, which affects syntax highlighting and accessibility.
As per coding guidelines.
Apply this diff:
**No loops:** -``` +```console 🔍 Checking for redirect loops... 📋 Found 538 internal redirects to check ✅ No redirect loops detected!Loops detected:
-+console
🔍 Checking for redirect loops...📋 Found 538 internal redirects to check
❌ Found 1 redirect loop(s):
Loop 1:
Chain: /getting-started → /docs/getting-started → /getting-startedAlso applies to: 53-63
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/check-redirects.yml(1 hunks)package.json(1 hunks)scripts/README.md(1 hunks)scripts/check-redirect-loops.js(1 hunks)
🧰 Additional context used
🪛 LanguageTool
scripts/README.md
[uncategorized] ~39-~39: The official name of this software platform is spelled with a capital “H”.
Context: ...nthat modifystatic/_redirects See.github/workflows/check-redirects.yml` for the ...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
scripts/README.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (1)
- GitHub Check: Check internal links
🔇 Additional comments (6)
scripts/check-redirect-loops.js (4)
1-13: LGTM! Clean setup with appropriate constants.The shebang, imports, and constants are well-defined. The max depth of 10 is reasonable for detecting infinite loops while allowing legitimate redirect chains.
58-94: LGTM! Solid loop detection algorithm.The redirect chain traversal correctly uses a visited set to detect cycles, handles max depth gracefully, and returns detailed loop information. The logic properly distinguishes between loops and legitimate chain endings.
96-148: LGTM! Well-structured main function with proper error handling.The function correctly orchestrates the loop checking process, uses the
checkedset optimization to avoid redundant analysis, provides clear console output, and exits with appropriate status codes for CI integration.
46-51: Order-dependent behavior exists but doesn't affect loop detection currently—clarify the actual code execution.The conditional block on lines 43-47 that adds
baseSourceto the redirects map never executes in practice because all base paths in the file appear as redirect sources before their hash-fragment variants (e.g., line 320 before line 322). When the hash-fragment source is processed,redirects.has(baseSource)is already true, so the assignment doesn't happen.While the order-dependency is technically real, the script correctly detects no loops with the current file structure. The concern about detection behavior depending on redirect order would only manifest if the file were reorganized to place hash-fragment sources before their base paths. The existing implementation is safe but fragile to future edits.
package.json (1)
17-17: LGTM! Clean npm script addition.The new
check-redirectsscript is well-named and correctly references the loop detection script..github/workflows/check-redirects.yml (1)
1-39: LGTM! Well-configured CI workflow.The workflow correctly:
- Triggers on changes to the redirects file
- Uses appropriate action versions
- Matches the Node.js version requirement from package.json (>= 18.0)
- Skips
npm installappropriately since the script only uses Node.js built-ins- Posts helpful PR comments when loops are detected
|
|
||
| for (const line of lines) { | ||
| // Skip comments, empty lines, and the footer | ||
| if (line.trim().startsWith('#') || line.trim() === '' || line.includes('NO REDIRECTS BELOW')) { |
There was a problem hiding this comment.
Consider more specific footer marker detection.
The condition line.includes('NO REDIRECTS BELOW') will match this text anywhere in the line. If a redirect path or comment contains this phrase, it will be incorrectly skipped.
Apply this diff to make the check more specific:
- if (line.trim().startsWith('#') || line.trim() === '' || line.includes('NO REDIRECTS BELOW')) {
+ if (line.trim().startsWith('#') || line.trim() === '' || line.trim().includes('# NO REDIRECTS BELOW')) {This ensures the marker is in a comment, not part of a redirect path.
🤖 Prompt for AI Agents
In scripts/check-redirect-loops.js around line 21, the current check uses
line.includes('NO REDIRECTS BELOW') which will match the phrase anywhere in the
line; change it to only detect the footer when it appears as a comment (e.g.,
after optional leading whitespace the line starts with a '#' followed by the
marker) so redirect paths or inline comments that contain the phrase are not
treated as the footer; implement this by trimming leading whitespace and testing
for a comment-start marker with the exact marker (or a regex like /^\s*#\s*NO
REDIRECTS BELOW\b/).
| - Pull requests that modify `static/_redirects` | ||
| - Pushes to `main` that modify `static/_redirects` | ||
|
|
||
| See `.github/workflows/check-redirects.yml` for the workflow configuration. |
There was a problem hiding this comment.
Fix capitalization of GitHub.
As per coding guidelines.
Apply this diff:
-See `.github/workflows/check-redirects.yml` for the workflow configuration.
+See `.github/workflows/check-redirects.yml` for the workflow configuration.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~39-~39: The official name of this software platform is spelled with a capital “H”.
Context: ...nthat modifystatic/_redirects See.github/workflows/check-redirects.yml` for the ...
(GITHUB)
🤖 Prompt for AI Agents
In scripts/README.md around line 39, the reference to GitHub should use the
proper capitalization; update the sentence to mention "GitHub" (capitalized)
when referring to the platform while keeping the repository path
`.github/workflows/check-redirects.yml` lowercase — e.g. reword to "See the
GitHub workflow configuration at `.github/workflows/check-redirects.yml`."
Summary by CodeRabbit
Chores