Skip to content

feat: add CI linter to detect redirect chains and stale content links#18620

Open
sfanahata wants to merge 10 commits into
masterfrom
lint/redirect-chains
Open

feat: add CI linter to detect redirect chains and stale content links#18620
sfanahata wants to merge 10 commits into
masterfrom
lint/redirect-chains

Conversation

@sfanahata

Copy link
Copy Markdown
Contributor

Summary

Adds a new GitHub Actions workflow and linting script that catches redirect chain issues on PRs before they get merged into production. This prevents the kind of multi-hop redirect buildup that required cleanup in #18606 and #18619.

What it detects

1. Redirect-to-redirect chains
Where redirect A's destination is another redirect B's source, creating multi-hop chains. Example:

/old/path/ -> /middle/path/ -> /final/path/
                ^^^ this destination is also a redirect source

2. Content links pointing to redirect sources
Where .mdx files contain internal links that hit a redirect instead of pointing directly to the final destination.

How it works

  • Reuses the existing parseRedirectsJs() and parseMiddlewareTs() parsers from check-redirects-on-rename.ts
  • Builds a unified redirect map from both middleware.ts and redirects.js
  • Phase 1: Checks if any redirect's destination exists as another redirect's source (simple string equality -- works for both exact-match and parameterized redirects like :path*, :platform, etc.)
  • Phase 2: Scans all .mdx files in docs/, develop-docs/, includes/, and platform-includes/ for internal links matching redirect sources
  • Posts/updates a PR comment with tables showing all issues found + suggested fixes

New files

File Lines Purpose
scripts/lint-redirect-chains.ts 333 Linter script
scripts/lint-redirect-chains.spec.ts 312 25 test cases
.github/workflows/lint-redirect-chains.yml 169 GitHub Actions workflow

Design decisions

  • Advisory, not blocking: continue-on-error: true -- marks the check as failed but does not block merge, matching the existing check-redirects-on-rename pattern
  • Path-filtered triggers: Only runs when middleware.ts, redirects.js, or content files change
  • No new dependencies: Uses only Node builtins (fs, path) and existing project deps (tsx, vitest)
  • Reuses existing parsers: Imports parseRedirectsJs and parseMiddlewareTs from check-redirects-on-rename.ts -- no duplicate parsing logic

PR comment format

When issues are found, the workflow posts a comment like:

⚠️ Redirect Chain Issues Detected

Redirect-to-Redirect Chains

Source Current Destination Should Be File
/old/ /middle/ /final/ redirects.js

Content Links Pointing to Redirects

File Line Current Link Should Be
docs/foo.mdx 42 /old/path/ /new/path/

Note

This PR should be merged after #18619 (cleanup of existing chains), so the linter starts clean against master.

Add a new GitHub Actions workflow and linting script that detects two
classes of redirect issues on PRs:

1. Redirect-to-redirect chains: where redirect A's destination is
   another redirect B's source, creating multi-hop chains that degrade
   SEO link equity (~15% loss per hop) and add user latency.

2. Content links pointing to redirect sources: where .mdx files contain
   internal links that hit a redirect instead of pointing directly to
   the final destination.

New files:
- scripts/lint-redirect-chains.ts: The linter script. Reuses the
  existing parseRedirectsJs() and parseMiddlewareTs() parsers from
  check-redirects-on-rename.ts. Builds a unified redirect map from
  both middleware.ts and redirects.js, then:
  - Phase 1: Checks if any redirect's destination is another redirect's
    source (simple string equality, works for both exact-match and
    parameterized redirects)
  - Phase 2: Scans all .mdx files in docs/, develop-docs/, includes/,
    and platform-includes/ for internal links matching redirect sources
  Outputs structured JSON for the workflow to parse.

- scripts/lint-redirect-chains.spec.ts: 25 test cases covering chain
  walking, cycle detection, final destination resolution, cross-system
  chain detection, content link regex patterns, and integration tests
  against the real redirect files.

- .github/workflows/lint-redirect-chains.yml: Runs on PRs to master
  when redirect files or content files change. Advisory only
  (continue-on-error: true) -- marks the check as failed but does not
  block merge. Posts/updates a PR comment with a table of all issues
  found, including suggested fixes.

- package.json: Adds lint:redirect-chains npm script.
@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
develop-docs Ready Ready Preview, Comment Jul 1, 2026 11:01pm
sentry-docs Ready Ready Preview, Comment Jul 1, 2026 11:01pm

Request Review

Comment thread scripts/lint-redirect-chains.ts Outdated
Comment thread scripts/lint-redirect-chains.ts
Shannon Anahata and others added 2 commits July 1, 2026 14:33
Fix TypeScript lint errors:
- Remove unused detectContentLinkIssues import from spec file
- Remove unused jsRedirects/mwRedirects variables in test

Fix trailing slash lookup mismatch (Cursor Bugbot):
- Some redirect sources in redirects.js omit trailing slashes (e.g.
  /processing-tickets instead of /processing-tickets/). The linter now
  checks both with and without trailing slash when looking up links
  against the redirect map.

Exclude PlatformLink to= attributes from content link scanning:
- PlatformLink prepends the current platform's base URL to its 'to'
  path, making them platform-relative (e.g. to="/profiling/" resolves
  to /platforms/android/profiling/). These should not be matched against
  the redirect map which contains absolute doc paths.
- Use negative lookbehind (?<!PlatformLink\s+) in the JSX to= regex
  to skip PlatformLink while still matching <Link to=>, etc.
- Add test cases for both PlatformLink exclusion and regular Link
  inclusion.
Comment thread scripts/lint-redirect-chains.ts Outdated
Comment thread scripts/lint-redirect-chains.ts Outdated
Comment thread scripts/lint-redirect-chains.ts Outdated
Replace the negative lookbehind (?<!PlatformLink\s+) approach with a
line-level check for '<PlatformLink'. The lookbehind was too narrow --
it only matched when 'to=' immediately followed 'PlatformLink', missing
cases where other props appear first (e.g. platform="android" to=).

The line-level check is simpler and more robust: if a line contains
'<PlatformLink' and the matched attribute is 'to', skip it.

Also simplify the regex setup from three separate patterns back to two
(markdownLinkRegex + jsxLinkRegex), with jsxLinkRegex now capturing the
attribute name in group 1 to enable the PlatformLink filtering.

Add test case for PlatformLink with props before to= attribute.
- 'includes/**/*.mdx'
- 'includes/**/*.md'
- 'platform-includes/**/*.mdx'
- 'platform-includes/**/*.md'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Workflow skips linter file changes

Medium Severity

The pull-request paths filter lists middleware.ts, redirects.js, and content globs only. Changes confined to scripts/lint-redirect-chains.ts, its spec, or this workflow file do not match, so the new job will not run on PRs that touch only the linter or CI wiring—including the PR that introduces it.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 523dff1. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The workflow intentionally only triggers on redirect/content file changes, because that's what it lints. Changes to the linter script itself are validated by the existing test suite (pnpm test / vitest). This matches the pattern of check-redirects-on-rename.yml. Adding the linter's own files to the path filter would just run the linter needlessly on PRs that only touch the linter code.

Comment thread .github/workflows/lint-redirect-chains.yml Outdated
Comment thread scripts/lint-redirect-chains.ts Outdated
The fallback single-parse would produce a string instead of an object,
causing the check to silently pass when the primary double-parse fails.
Replace with explicit error reporting via core.setFailed().
1. Use resolveToFinal for final destination instead of walkChain's last
   element. walkChain caps at maxDepth=10 for display, so its last
   element could be an intermediate hop. resolveToFinal has no depth
   limit and correctly resolves to the true terminal destination.

2. Handle duplicate sources across files properly. Instead of merging
   all redirects and deduplicating with seenSources (which silently
   dropped one entry), process each file's redirects separately with
   correct file attribution. Both entries are reported when the same
   source exists in both redirects.js and middleware.ts, since that's
   itself a problem worth flagging.

3. Preserve URL fragments in content link suggestions. Links like
   /old/path/#section now produce suggestions like /new/path/#section
   instead of dropping the fragment. The ContentLinkIssue interface
   now includes a fragment field, and both linkPath and finalDest
   include the fragment in their output.

4. Update all regex patterns in tests to match the new capturing group
   format (fragment is now captured separately instead of discarded).
   Add test for duplicate source detection across files.
Comment thread scripts/lint-redirect-chains.ts Outdated
Comment thread .github/workflows/lint-redirect-chains.yml
Shannon Anahata added 2 commits July 1, 2026 15:28
When the same source exists in both redirects.js and middleware.ts with
different destinations, walking from r.source through the unified map
would use the other file's destination (last writer wins in the Map),
creating a mismatch between currentDest and the chain.

Fix: walk from r.destination (the entry's own next hop) instead of
r.source. The chain is now [source, ...walkChain(destination)], which
guarantees chain[1] === currentDest. Similarly, resolveToFinal starts
from r.destination so finalDest is consistent with the entry's path.

Update duplicate source test to verify chain[1] matches currentDest
and finalDest follows each file's own redirect path.
The GitHub API returns 30 comments per page by default. On PRs with
30+ comments, the bot comment could be on a later page, causing
listComments to miss it and post a duplicate. Use github.paginate()
to fetch all comments.

Also tighten the bot login check from loose .includes('bot') to
exact match 'github-actions[bot]'.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b4ee4f6. Configure here.

Comment thread scripts/lint-redirect-chains.ts Outdated
Shannon Anahata added 2 commits July 1, 2026 15:42
Same bug: listComments returns 30 per page by default, so on busy PRs
the bot comment lookup would miss existing comments and post duplicates.
Apply the same github.paginate() fix and tighten the bot login check.
Phase 2 already normalized trailing slashes when looking up content
links, but Phase 1 did not. If a redirect destination like '/middle'
(no slash) chains into a source like '/middle/' (with slash), the
chain would go undetected.

Fix: when building the redirect map, index sources both with and
without trailing slash. When checking if a destination chains, try
both the raw destination and the normalized form.

No mismatches exist in the current data, but this prevents future
ones from slipping through.
@sfanahata sfanahata requested review from coolguyzone and sergical July 2, 2026 19:33
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.

2 participants