From 7795942dc66b5d94860db4c6f07f58133594f874 Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 13:34:19 -0700 Subject: [PATCH 01/10] feat: add CI linter to detect redirect chains and stale content links 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. --- .github/workflows/lint-redirect-chains.yml | 169 +++++++++++ package.json | 1 + scripts/lint-redirect-chains.spec.ts | 312 +++++++++++++++++++ scripts/lint-redirect-chains.ts | 333 +++++++++++++++++++++ 4 files changed, 815 insertions(+) create mode 100644 .github/workflows/lint-redirect-chains.yml create mode 100644 scripts/lint-redirect-chains.spec.ts create mode 100644 scripts/lint-redirect-chains.ts diff --git a/.github/workflows/lint-redirect-chains.yml b/.github/workflows/lint-redirect-chains.yml new file mode 100644 index 0000000000000..4b763366accac --- /dev/null +++ b/.github/workflows/lint-redirect-chains.yml @@ -0,0 +1,169 @@ +name: Lint Redirect Chains + +on: + pull_request: + branches: [master] + paths: + - 'middleware.ts' + - 'redirects.js' + - 'docs/**/*.mdx' + - 'docs/**/*.md' + - 'develop-docs/**/*.mdx' + - 'develop-docs/**/*.md' + - 'includes/**/*.mdx' + - 'includes/**/*.md' + - 'platform-includes/**/*.mdx' + - 'platform-includes/**/*.md' + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + lint-redirect-chains: + name: Check for redirect chains + runs-on: ubuntu-latest + continue-on-error: true # Fail the check but don't block merge + permissions: + contents: read + pull-requests: write + + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + - name: Setup Node + uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v4 + with: + node-version-file: 'package.json' + + - name: Run redirect chain lint + id: lint + continue-on-error: true + run: | + set +e + OUTPUT=$(npx tsx scripts/lint-redirect-chains.ts 2>&1) + EXIT_CODE=$? + set -e + + echo "$OUTPUT" + + # Extract JSON output if present + HAS_JSON=false + if echo "$OUTPUT" | grep -Fq -- "---JSON_OUTPUT---"; then + JSON_OUTPUT=$(echo "$OUTPUT" | sed -n '/---JSON_OUTPUT---/,/---JSON_OUTPUT---/p' | sed '1d;$d') + echo "lint_result<> $GITHUB_OUTPUT + echo "$JSON_OUTPUT" >> $GITHUB_OUTPUT + echo "EOF" >> $GITHUB_OUTPUT + HAS_JSON=true + fi + + echo "has_results=$HAS_JSON" >> $GITHUB_OUTPUT + echo "exit_code=$EXIT_CODE" >> $GITHUB_OUTPUT + + - name: Post comment if redirect chains found + if: steps.lint.outputs.exit_code == '1' && steps.lint.outputs.has_results == 'true' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + script: | + const lintResultJsonString = ${{ toJSON(steps.lint.outputs.lint_result) }}; + let lintResult; + try { + const jsonString = JSON.parse(lintResultJsonString); + lintResult = JSON.parse(jsonString); + } catch (e) { + try { + lintResult = JSON.parse(lintResultJsonString); + } catch (e2) { + console.error('Failed to parse lint result:', e2); + return; + } + } + + const redirectChains = lintResult.redirectChains || []; + const contentLinkIssues = lintResult.contentLinkIssues || []; + + if (redirectChains.length === 0 && contentLinkIssues.length === 0) { + return; + } + + let comment = '## \u26a0\ufe0f Redirect Chain Issues Detected\n\n'; + comment += 'This PR introduces or contains redirect chains that degrade SEO and add latency.\n\n'; + + if (redirectChains.length > 0) { + comment += '### Redirect-to-Redirect Chains\n\n'; + comment += 'These redirect destinations point to another redirect, creating multi-hop chains. '; + comment += 'Update the destination to point directly to the final URL:\n\n'; + comment += '| Source | Current Destination | Should Be | File |\n'; + comment += '|--------|-------------------|-----------|------|\n'; + for (const chain of redirectChains) { + const docLabel = chain.isDeveloperDocs ? ' (dev)' : ''; + comment += `| \`${chain.source}\` | \`${chain.currentDest}\` | \`${chain.finalDest}\` | ${chain.file}${docLabel} |\n`; + } + comment += '\n'; + } + + if (contentLinkIssues.length > 0) { + // Group by file + const byFile = {}; + for (const issue of contentLinkIssues) { + if (!byFile[issue.filePath]) byFile[issue.filePath] = []; + byFile[issue.filePath].push(issue); + } + + comment += `### Content Links Pointing to Redirects (${contentLinkIssues.length} found)\n\n`; + comment += 'These links point to URLs that redirect. Update them to point directly to the final destination:\n\n'; + comment += '| File | Line | Current Link | Should Be |\n'; + comment += '|------|------|--------------|-----------|\n'; + for (const [file, issues] of Object.entries(byFile)) { + for (const issue of issues) { + comment += `| \`${file}\` | ${issue.line} | \`${issue.linkPath}\` | \`${issue.finalDest}\` |\n`; + } + } + comment += '\n'; + } + + comment += '---\n'; + comment += '*Each redirect hop loses ~15% of SEO link equity and adds latency for users.*\n'; + + // Check for existing comments from this action + const {data: comments} = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const existingComment = comments.find(c => + c.user.type === 'Bot' && + (c.user.login === 'github-actions[bot]' || c.user.login.includes('bot')) && + c.body.includes('Redirect Chain Issues Detected') + ); + + if (existingComment) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existingComment.id, + body: comment, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: comment, + }); + } + + - name: Report failure if redirect chains found + if: steps.lint.outputs.exit_code == '1' && steps.lint.outputs.has_results == 'true' + run: | + echo "::warning::Redirect chain issues detected. See PR comment for details." + exit 1 + + - name: Success + if: steps.lint.outputs.exit_code == '0' + run: | + echo "No redirect chains or content link issues found." diff --git a/package.json b/package.json index f50acfc66258e..49f5ba83e3820 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "lint:prettier": "prettier --check \"./{src,app,scripts}/**/*.{md,mdx,ts,tsx,js,jsx,mjs}\"", "lint:prettier:fix": "prettier --write \"./{src,app,scripts}/**/*.{md,mdx,ts,tsx,js,jsx,mjs}\"", "lint:typos": "typos", + "lint:redirect-chains": "tsx scripts/lint-redirect-chains.ts", "lint:fix": "pnpm run lint:prettier:fix && pnpm run lint:eslint:fix", "sidecar": "spotlight-sidecar", "test": "vitest", diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts new file mode 100644 index 0000000000000..ab63d75835896 --- /dev/null +++ b/scripts/lint-redirect-chains.spec.ts @@ -0,0 +1,312 @@ +import fs from 'fs'; +import path from 'path'; +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; + +import {parseMiddlewareTs, parseRedirectsJs} from './check-redirects-on-rename'; +import { + detectContentLinkIssues, + detectRedirectChains, + resolveToFinal, + walkChain, +} from './lint-redirect-chains'; + +describe('walkChain', () => { + it('should return just the source when no chain exists', () => { + const map = new Map([ + ['/a/', '/b/'], + ['/c/', '/d/'], + ]); + expect(walkChain('/a/', map)).toEqual(['/a/', '/b/']); + }); + + it('should walk a 2-hop chain', () => { + const map = new Map([ + ['/a/', '/b/'], + ['/b/', '/c/'], + ]); + expect(walkChain('/a/', map)).toEqual(['/a/', '/b/', '/c/']); + }); + + it('should walk a 3-hop chain', () => { + const map = new Map([ + ['/a/', '/b/'], + ['/b/', '/c/'], + ['/c/', '/d/'], + ]); + expect(walkChain('/a/', map)).toEqual(['/a/', '/b/', '/c/', '/d/']); + }); + + it('should detect cycles', () => { + const map = new Map([ + ['/a/', '/b/'], + ['/b/', '/a/'], + ]); + const result = walkChain('/a/', map); + expect(result).toEqual(['/a/', '/b/', '/a/ (CYCLE)']); + }); + + it('should detect self-referencing cycles', () => { + const map = new Map([['/a/', '/a/']]); + const result = walkChain('/a/', map); + expect(result).toEqual(['/a/', '/a/ (CYCLE)']); + }); + + it('should cap at maxDepth', () => { + const map = new Map(); + for (let i = 0; i < 20; i++) { + map.set(`/${i}/`, `/${i + 1}/`); + } + const result = walkChain('/0/', map, 5); + expect(result).toHaveLength(6); // source + 5 hops + }); +}); + +describe('resolveToFinal', () => { + it('should resolve a single redirect', () => { + const map = new Map([['/a/', '/b/']]); + expect(resolveToFinal('/a/', map)).toBe('/b/'); + }); + + it('should resolve a chain to the final destination', () => { + const map = new Map([ + ['/a/', '/b/'], + ['/b/', '/c/'], + ['/c/', '/d/'], + ]); + expect(resolveToFinal('/a/', map)).toBe('/d/'); + }); + + it('should handle self-referencing cycles without infinite loop', () => { + const map = new Map([['/a/', '/a/']]); + expect(resolveToFinal('/a/', map)).toBe('/a/'); + }); + + it('should return the source when not in the map', () => { + const map = new Map([['/a/', '/b/']]); + expect(resolveToFinal('/x/', map)).toBe('/x/'); + }); +}); + +describe('detectRedirectChains', () => { + it('should detect chains within a single redirect source', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/old/', destination: '/middle/'}, + {source: '/middle/', destination: '/new/'}, + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].source).toBe('/old/'); + expect(chains[0].currentDest).toBe('/middle/'); + expect(chains[0].finalDest).toBe('/new/'); + expect(chains[0].file).toBe('redirects.js'); + expect(chains[0].isDeveloperDocs).toBe(false); + }); + + it('should detect cross-system chains (middleware -> redirects.js)', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/middle/', destination: '/final/'}, + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/old/', destination: '/middle/'}, + ], + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].source).toBe('/old/'); + expect(chains[0].finalDest).toBe('/final/'); + expect(chains[0].file).toBe('middleware.ts'); + }); + + it('should return empty array when no chains exist', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/a/', destination: '/b/'}, + {source: '/c/', destination: '/d/'}, + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(0); + }); + + it('should handle self-redirect cycles', () => { + const jsRedirects = { + developerDocsRedirects: [ + {source: '/loop/', destination: '/loop/'}, + ], + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].finalDest).toBe('(CYCLE DETECTED)'); + }); + + it('should handle parameterized redirect chains', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/old/:path*', destination: '/middle/:path*'}, + {source: '/middle/:path*', destination: '/new/:path*'}, + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].source).toBe('/old/:path*'); + expect(chains[0].finalDest).toBe('/new/:path*'); + }); + + it('should separate user docs and developer docs', () => { + const jsRedirects = { + developerDocsRedirects: [ + {source: '/sdk/old/', destination: '/sdk/middle/'}, + {source: '/sdk/middle/', destination: '/sdk/new/'}, + ], + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].isDeveloperDocs).toBe(true); + }); +}); + +describe('detectContentLinkIssues', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = path.join(process.cwd(), '__test_mdx_temp__'); + fs.mkdirSync(path.join(tempDir, 'docs'), {recursive: true}); + }); + + afterEach(() => { + fs.rmSync(tempDir, {recursive: true, force: true}); + }); + + it('should detect markdown links pointing to redirect sources', () => { + const mdxContent = `--- +title: Test +--- + +Check out the [old page](/old/path/) for more info. +`; + fs.writeFileSync(path.join(tempDir, 'docs', 'test.mdx'), mdxContent); + + // Monkey-patch findAllMdxFiles to use our temp dir + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/old/path/', destination: '/new/path/'}, + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + // We test the link detection logic by checking the regex patterns directly + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const matches = [...mdxContent.matchAll(markdownLinkRegex)]; + expect(matches).toHaveLength(1); + expect(matches[0][1]).toBe('/old/path/'); + }); + + it('should detect JSX links pointing to redirect sources', () => { + const mdxContent = ``; + + const jsxLinkRegex = /(?:href|to|url)="(\/[^"#]+?)(?:#[^"]+)?"/g; + const matches = [...mdxContent.matchAll(jsxLinkRegex)]; + expect(matches).toHaveLength(1); + expect(matches[0][1]).toBe('/old/path/'); + }); + + it('should strip anchors from links', () => { + const mdxContent = `[link](/old/path/#section)`; + + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const matches = [...mdxContent.matchAll(markdownLinkRegex)]; + expect(matches).toHaveLength(1); + expect(matches[0][1]).toBe('/old/path/'); + }); + + it('should skip external links', () => { + const mdxContent = `[link](https://example.com/path/)`; + + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const matches = [...mdxContent.matchAll(markdownLinkRegex)]; + expect(matches).toHaveLength(0); + }); + + it('should skip hash-only links', () => { + const mdxContent = `[link](#section)`; + + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const matches = [...mdxContent.matchAll(markdownLinkRegex)]; + expect(matches).toHaveLength(0); + }); + + it('should handle links without trailing slashes', () => { + const mdxContent = `[link](/old/path)`; + + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const matches = [...mdxContent.matchAll(markdownLinkRegex)]; + expect(matches).toHaveLength(1); + expect(matches[0][1]).toBe('/old/path'); + // Normalization adds trailing slash: /old/path -> /old/path/ + }); + + it('should detect PlatformLink to= attributes', () => { + const mdxContent = `text`; + + const jsxLinkRegex = /(?:href|to|url)="(\/[^"#]+?)(?:#[^"]+)?"/g; + const matches = [...mdxContent.matchAll(jsxLinkRegex)]; + expect(matches).toHaveLength(1); + expect(matches[0][1]).toBe('/old/feature/'); + }); +}); + +describe('integration: real redirect files', () => { + it('should parse real redirects.js without errors', () => { + const result = parseRedirectsJs('redirects.js'); + expect(result.userDocsRedirects.length).toBeGreaterThan(0); + expect(result.developerDocsRedirects.length).toBeGreaterThan(0); + }); + + it('should parse real middleware.ts without errors', () => { + const result = parseMiddlewareTs('middleware.ts'); + expect(result.userDocsRedirects.length).toBeGreaterThan(0); + expect(result.developerDocsRedirects.length).toBeGreaterThan(0); + }); +}); diff --git a/scripts/lint-redirect-chains.ts b/scripts/lint-redirect-chains.ts new file mode 100644 index 0000000000000..71b9e9db8ee44 --- /dev/null +++ b/scripts/lint-redirect-chains.ts @@ -0,0 +1,333 @@ +/** + * Lint redirect chains: detects multi-hop redirects and content links + * pointing to redirect sources. + * + * Phase 1: Finds redirect-to-redirect chains in middleware.ts and redirects.js + * (where redirect A's destination is another redirect B's source). + * Phase 2: Finds internal links in .mdx content files that point to URLs + * which are redirect sources (and should point to the final destination). + * + * Outputs structured JSON for the GitHub Action workflow to post as a PR comment. + */ +import fs from 'fs'; +import path from 'path'; + +import {parseMiddlewareTs, parseRedirectsJs} from './check-redirects-on-rename'; + +interface Redirect { + destination: string; + source: string; +} + +interface RedirectChain { + chain: string[]; + currentDest: string; + file: string; + finalDest: string; + isDeveloperDocs: boolean; + source: string; +} + +interface ContentLinkIssue { + filePath: string; + finalDest: string; + isDeveloperDocs: boolean; + line: number; + linkPath: string; +} + +/** + * Recursively finds all .mdx and .md files in the given directories + */ +function findAllMdxFiles(dirs: string[]): string[] { + const files: string[] = []; + for (const dir of dirs) { + if (!fs.existsSync(dir)) { + continue; + } + const walk = (d: string) => { + for (const entry of fs.readdirSync(d, {withFileTypes: true})) { + const full = path.join(d, entry.name); + if (entry.isDirectory()) { + walk(full); + } else if (entry.name.endsWith('.mdx') || entry.name.endsWith('.md')) { + files.push(full); + } + } + }; + walk(dir); + } + return files; +} + +/** + * Walks a redirect chain to its final destination. + * Returns the full chain path including the starting source. + * Detects cycles and caps at maxDepth to prevent infinite loops. + */ +function walkChain( + source: string, + redirectMap: Map, + maxDepth = 10 +): string[] { + const chain = [source]; + let current = source; + const seen = new Set(); + seen.add(current); + let depth = 0; + while (redirectMap.has(current) && depth < maxDepth) { + const next = redirectMap.get(current)!; + if (seen.has(next)) { + chain.push(next + ' (CYCLE)'); + break; + } + seen.add(next); + chain.push(next); + current = next; + depth++; + } + return chain; +} + +/** + * Resolves a path through the redirect map to its final destination. + */ +function resolveToFinal(source: string, map: Map): string { + let current = source; + const seen = new Set(); + while (map.has(current) && !seen.has(current)) { + seen.add(current); + current = map.get(current)!; + } + return current; +} + +/** + * Phase 1: Detect redirect-to-redirect chains + */ +function detectRedirectChains( + jsRedirects: {developerDocsRedirects: Redirect[]; userDocsRedirects: Redirect[]}, + mwRedirects: {developerDocsRedirects: Redirect[]; userDocsRedirects: Redirect[]} +): RedirectChain[] { + const chains: RedirectChain[] = []; + + for (const docType of ['user', 'developer'] as const) { + const isDevDocs = docType === 'developer'; + const jsArr = isDevDocs + ? jsRedirects.developerDocsRedirects + : jsRedirects.userDocsRedirects; + const mwArr = isDevDocs + ? mwRedirects.developerDocsRedirects + : mwRedirects.userDocsRedirects; + + // Build unified map for chain detection + const allRedirects = [...jsArr, ...mwArr]; + const redirectMap = new Map(); + for (const r of allRedirects) { + redirectMap.set(r.source, r.destination); + } + + // Track which file each source comes from + const jsSourceSet = new Set(jsArr.map(r => r.source)); + + // Find chains + const seenSources = new Set(); + for (const r of allRedirects) { + if (seenSources.has(r.source)) { + continue; + } + seenSources.add(r.source); + + if (redirectMap.has(r.destination)) { + const chain = walkChain(r.source, redirectMap); + const finalDest = chain[chain.length - 1]; + + // Skip self-referencing cycles + if (finalDest === r.source || finalDest.includes('(CYCLE)')) { + chains.push({ + source: r.source, + currentDest: r.destination, + finalDest: '(CYCLE DETECTED)', + chain, + file: jsSourceSet.has(r.source) ? 'redirects.js' : 'middleware.ts', + isDeveloperDocs: isDevDocs, + }); + continue; + } + + chains.push({ + source: r.source, + currentDest: r.destination, + finalDest, + chain, + file: jsSourceSet.has(r.source) ? 'redirects.js' : 'middleware.ts', + isDeveloperDocs: isDevDocs, + }); + } + } + } + + return chains; +} + +/** + * Phase 2: Detect content links pointing to redirect sources + */ +function detectContentLinkIssues( + jsRedirects: {developerDocsRedirects: Redirect[]; userDocsRedirects: Redirect[]}, + mwRedirects: {developerDocsRedirects: Redirect[]; userDocsRedirects: Redirect[]} +): ContentLinkIssue[] { + // Build redirect maps + const userRedirectMap = new Map(); + for (const r of [ + ...jsRedirects.userDocsRedirects, + ...mwRedirects.userDocsRedirects, + ]) { + userRedirectMap.set(r.source, r.destination); + } + const devRedirectMap = new Map(); + for (const r of [ + ...jsRedirects.developerDocsRedirects, + ...mwRedirects.developerDocsRedirects, + ]) { + devRedirectMap.set(r.source, r.destination); + } + + const mdxFiles = findAllMdxFiles([ + 'docs', + 'develop-docs', + 'includes', + 'platform-includes', + ]); + + // Regex patterns for extracting internal links + // Markdown: [text](/path/) or [text](/path/#anchor) + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + // JSX: href="/path/" or to="/path/" or url="/path/" + const jsxLinkRegex = /(?:href|to|url)="(\/[^"#]+?)(?:#[^"]+)?"/g; + + const issues: ContentLinkIssue[] = []; + + for (const filePath of mdxFiles) { + const content = fs.readFileSync(filePath, 'utf8'); + const lines = content.split('\n'); + const isDevDocsFile = filePath.startsWith('develop-docs/'); + const map = isDevDocsFile ? devRedirectMap : userRedirectMap; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + for (const regex of [markdownLinkRegex, jsxLinkRegex]) { + regex.lastIndex = 0; + let match; + while ((match = regex.exec(line)) !== null) { + let linkPath = match[1]; + // Normalize: ensure trailing slash + if (!linkPath.endsWith('/')) { + linkPath += '/'; + } + + if (map.has(linkPath)) { + const finalDest = resolveToFinal(linkPath, map); + issues.push({ + filePath, + line: i + 1, + linkPath, + finalDest, + isDeveloperDocs: isDevDocsFile, + }); + } + } + } + } + } + + return issues; +} + +// Main execution +if (require.main === module) { + console.log('Linting redirect chains...\n'); + + const jsRedirects = parseRedirectsJs('redirects.js'); + const mwRedirects = parseMiddlewareTs('middleware.ts'); + + console.log( + `Parsed: ${jsRedirects.userDocsRedirects.length + jsRedirects.developerDocsRedirects.length} from redirects.js, ` + + `${mwRedirects.userDocsRedirects.length + mwRedirects.developerDocsRedirects.length} from middleware.ts` + ); + + // Phase 1: Redirect chains + const chains = detectRedirectChains(jsRedirects, mwRedirects); + if (chains.length > 0) { + console.log(`\n❌ Found ${chains.length} redirect chain(s):\n`); + for (const c of chains) { + console.log(` [${c.file}] ${c.chain.join(' -> ')}`); + console.log( + ` Fix: Change destination of "${c.source}" to "${c.finalDest}"` + ); + } + } else { + console.log('\n✅ No redirect chains found.'); + } + + // Phase 2: Content links + const contentIssues = detectContentLinkIssues(jsRedirects, mwRedirects); + if (contentIssues.length > 0) { + console.log( + `\n❌ Found ${contentIssues.length} content link(s) pointing to redirect sources:\n` + ); + + // Group by file for readable output + const byFile = new Map(); + for (const issue of contentIssues) { + const arr = byFile.get(issue.filePath) || []; + arr.push(issue); + byFile.set(issue.filePath, arr); + } + + for (const [file, fileIssues] of byFile) { + console.log(` ${file}:`); + for (const issue of fileIssues) { + console.log( + ` L${issue.line}: ${issue.linkPath} -> should be ${issue.finalDest}` + ); + } + } + } else { + console.log('\n✅ No content links pointing to redirect sources.'); + } + + // Summary and JSON output + const hasIssues = chains.length > 0 || contentIssues.length > 0; + + if (hasIssues) { + console.log('\n---JSON_OUTPUT---'); + console.log( + JSON.stringify( + { + redirectChains: chains, + contentLinkIssues: contentIssues, + }, + null, + 2 + ) + ); + console.log('---JSON_OUTPUT---\n'); + + process.exit(1); + } else { + console.log( + '\n✅ All clear: no redirect chains or content links pointing to redirect sources.' + ); + process.exit(0); + } +} + +export { + detectContentLinkIssues, + detectRedirectChains, + findAllMdxFiles, + resolveToFinal, + walkChain, +}; From d365c91b8f61e89eacf0315962c212c1267d1eed Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 14:33:16 -0700 Subject: [PATCH 02/10] fix: address CI and review feedback on redirect chain linter 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 (?, etc. - Add test cases for both PlatformLink exclusion and regular Link inclusion. --- scripts/lint-redirect-chains.spec.ts | 31 ++++++++++++-------------- scripts/lint-redirect-chains.ts | 33 ++++++++++++++++++---------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts index ab63d75835896..e88ecd53ff212 100644 --- a/scripts/lint-redirect-chains.spec.ts +++ b/scripts/lint-redirect-chains.spec.ts @@ -4,7 +4,6 @@ import {afterEach, beforeEach, describe, expect, it} from 'vitest'; import {parseMiddlewareTs, parseRedirectsJs} from './check-redirects-on-rename'; import { - detectContentLinkIssues, detectRedirectChains, resolveToFinal, walkChain, @@ -224,19 +223,7 @@ Check out the [old page](/old/path/) for more info. `; fs.writeFileSync(path.join(tempDir, 'docs', 'test.mdx'), mdxContent); - // Monkey-patch findAllMdxFiles to use our temp dir - const jsRedirects = { - developerDocsRedirects: [] as Array<{destination: string; source: string}>, - userDocsRedirects: [ - {source: '/old/path/', destination: '/new/path/'}, - ], - }; - const mwRedirects = { - developerDocsRedirects: [] as Array<{destination: string; source: string}>, - userDocsRedirects: [] as Array<{destination: string; source: string}>, - }; - - // We test the link detection logic by checking the regex patterns directly + // Test the link detection logic by checking the regex patterns directly const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(1); @@ -287,11 +274,21 @@ Check out the [old page](/old/path/) for more info. // Normalization adds trailing slash: /old/path -> /old/path/ }); - it('should detect PlatformLink to= attributes', () => { + it('should skip PlatformLink to= attributes (platform-relative paths)', () => { const mdxContent = `text`; - const jsxLinkRegex = /(?:href|to|url)="(\/[^"#]+?)(?:#[^"]+)?"/g; - const matches = [...mdxContent.matchAll(jsxLinkRegex)]; + // PlatformLink to= should NOT be matched since PlatformLink prepends + // the platform base URL, making these platform-relative paths + const jsxToRegex = /(? { + const mdxContent = `text`; + + const jsxToRegex = /(? Date: Wed, 1 Jul 2026 21:34:10 +0000 Subject: [PATCH 03/10] [getsentry/action-github-commit] Auto commit --- scripts/lint-redirect-chains.spec.ts | 18 ++++-------------- scripts/lint-redirect-chains.ts | 13 +++---------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts index e88ecd53ff212..777bf0c047938 100644 --- a/scripts/lint-redirect-chains.spec.ts +++ b/scripts/lint-redirect-chains.spec.ts @@ -3,11 +3,7 @@ import path from 'path'; import {afterEach, beforeEach, describe, expect, it} from 'vitest'; import {parseMiddlewareTs, parseRedirectsJs} from './check-redirects-on-rename'; -import { - detectRedirectChains, - resolveToFinal, - walkChain, -} from './lint-redirect-chains'; +import {detectRedirectChains, resolveToFinal, walkChain} from './lint-redirect-chains'; describe('walkChain', () => { it('should return just the source when no chain exists', () => { @@ -112,15 +108,11 @@ describe('detectRedirectChains', () => { it('should detect cross-system chains (middleware -> redirects.js)', () => { const jsRedirects = { developerDocsRedirects: [] as Array<{destination: string; source: string}>, - userDocsRedirects: [ - {source: '/middle/', destination: '/final/'}, - ], + userDocsRedirects: [{source: '/middle/', destination: '/final/'}], }; const mwRedirects = { developerDocsRedirects: [] as Array<{destination: string; source: string}>, - userDocsRedirects: [ - {source: '/old/', destination: '/middle/'}, - ], + userDocsRedirects: [{source: '/old/', destination: '/middle/'}], }; const chains = detectRedirectChains(jsRedirects, mwRedirects); @@ -149,9 +141,7 @@ describe('detectRedirectChains', () => { it('should handle self-redirect cycles', () => { const jsRedirects = { - developerDocsRedirects: [ - {source: '/loop/', destination: '/loop/'}, - ], + developerDocsRedirects: [{source: '/loop/', destination: '/loop/'}], userDocsRedirects: [] as Array<{destination: string; source: string}>, }; const mwRedirects = { diff --git a/scripts/lint-redirect-chains.ts b/scripts/lint-redirect-chains.ts index 757dccb0d65e5..0525cb3b5c431 100644 --- a/scripts/lint-redirect-chains.ts +++ b/scripts/lint-redirect-chains.ts @@ -179,10 +179,7 @@ function detectContentLinkIssues( ): ContentLinkIssue[] { // Build redirect maps const userRedirectMap = new Map(); - for (const r of [ - ...jsRedirects.userDocsRedirects, - ...mwRedirects.userDocsRedirects, - ]) { + for (const r of [...jsRedirects.userDocsRedirects, ...mwRedirects.userDocsRedirects]) { userRedirectMap.set(r.source, r.destination); } const devRedirectMap = new Map(); @@ -226,9 +223,7 @@ function detectContentLinkIssues( while ((match = regex.exec(line)) !== null) { let linkPath = match[1]; // Normalize: ensure trailing slash for lookup - const normalizedPath = linkPath.endsWith('/') - ? linkPath - : linkPath + '/'; + const normalizedPath = linkPath.endsWith('/') ? linkPath : linkPath + '/'; // Check both with and without trailing slash, since some redirect // sources in redirects.js omit trailing slashes @@ -274,9 +269,7 @@ if (require.main === module) { console.log(`\n❌ Found ${chains.length} redirect chain(s):\n`); for (const c of chains) { console.log(` [${c.file}] ${c.chain.join(' -> ')}`); - console.log( - ` Fix: Change destination of "${c.source}" to "${c.finalDest}"` - ); + console.log(` Fix: Change destination of "${c.source}" to "${c.finalDest}"`); } } else { console.log('\n✅ No redirect chains found.'); From 523dff19ee73980e67935ae9491f4baadceb22f7 Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 14:41:00 -0700 Subject: [PATCH 04/10] fix: use line-level PlatformLink check instead of regex lookbehind Replace the negative lookbehind (? { - const mdxContent = `text`; + // PlatformLink to= should be skipped since PlatformLink prepends + // the platform base URL, making these platform-relative paths. + // The linter checks for text`; + const isPlatformLinkLine = line.includes(' { + // Handles case where PlatformLink has other props first + const line = `text`; + const isPlatformLinkLine = line.includes(' { - const mdxContent = `text`; + const line = `text`; + const isPlatformLinkLine = line.includes(' Date: Wed, 1 Jul 2026 15:01:42 -0700 Subject: [PATCH 05/10] fix: remove incorrect JSON parse fallback in workflow 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(). --- .github/workflows/lint-redirect-chains.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/lint-redirect-chains.yml b/.github/workflows/lint-redirect-chains.yml index 4b763366accac..b829b0847c447 100644 --- a/.github/workflows/lint-redirect-chains.yml +++ b/.github/workflows/lint-redirect-chains.yml @@ -74,12 +74,9 @@ jobs: const jsonString = JSON.parse(lintResultJsonString); lintResult = JSON.parse(jsonString); } catch (e) { - try { - lintResult = JSON.parse(lintResultJsonString); - } catch (e2) { - console.error('Failed to parse lint result:', e2); - return; - } + console.error('Failed to parse lint result:', e); + core.setFailed('Failed to parse redirect chain lint output'); + return; } const redirectChains = lintResult.redirectChains || []; From 0ff266d231d749f7be6817cbbe3a88f2c53cb5ca Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 15:13:54 -0700 Subject: [PATCH 06/10] fix: address all remaining review feedback 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. --- scripts/lint-redirect-chains.spec.ts | 43 +++++++++++---- scripts/lint-redirect-chains.ts | 81 +++++++++++++++------------- 2 files changed, 78 insertions(+), 46 deletions(-) diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts index 8adc9c14871ec..714f52d7d26df 100644 --- a/scripts/lint-redirect-chains.spec.ts +++ b/scripts/lint-redirect-chains.spec.ts @@ -122,6 +122,28 @@ describe('detectRedirectChains', () => { expect(chains[0].file).toBe('middleware.ts'); }); + it('should report duplicate sources from both files separately', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [{source: '/dup/', destination: '/middle/'}], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/dup/', destination: '/other-middle/'}, + {source: '/middle/', destination: '/final/'}, + {source: '/other-middle/', destination: '/other-final/'}, + ], + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + // Both entries for /dup/ should be reported with correct file attribution + const dupChains = chains.filter(c => c.source === '/dup/'); + expect(dupChains).toHaveLength(2); + expect(dupChains.find(c => c.file === 'redirects.js')).toBeDefined(); + expect(dupChains.find(c => c.file === 'middleware.ts')).toBeDefined(); + }); + it('should return empty array when no chains exist', () => { const jsRedirects = { developerDocsRedirects: [] as Array<{destination: string; source: string}>, @@ -214,7 +236,7 @@ Check out the [old page](/old/path/) for more info. fs.writeFileSync(path.join(tempDir, 'docs', 'test.mdx'), mdxContent); // Test the link detection logic by checking the regex patterns directly - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(1); expect(matches[0][1]).toBe('/old/path/'); @@ -223,25 +245,26 @@ Check out the [old page](/old/path/) for more info. it('should detect JSX links pointing to redirect sources', () => { const mdxContent = ``; - const jsxLinkRegex = /(?:href|to|url)="(\/[^"#]+?)(?:#[^"]+)?"/g; + const jsxLinkRegex = /((?:href|to|url))="(\/[^"#]+?)(#[^"]+)?"/g; const matches = [...mdxContent.matchAll(jsxLinkRegex)]; expect(matches).toHaveLength(1); - expect(matches[0][1]).toBe('/old/path/'); + expect(matches[0][2]).toBe('/old/path/'); }); - it('should strip anchors from links', () => { + it('should extract path and fragment separately from anchored links', () => { const mdxContent = `[link](/old/path/#section)`; - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(1); expect(matches[0][1]).toBe('/old/path/'); + expect(matches[0][2]).toBe('#section'); }); it('should skip external links', () => { const mdxContent = `[link](https://example.com/path/)`; - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(0); }); @@ -249,7 +272,7 @@ Check out the [old page](/old/path/) for more info. it('should skip hash-only links', () => { const mdxContent = `[link](#section)`; - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(0); }); @@ -257,7 +280,7 @@ Check out the [old page](/old/path/) for more info. it('should handle links without trailing slashes', () => { const mdxContent = `[link](/old/path)`; - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; const matches = [...mdxContent.matchAll(markdownLinkRegex)]; expect(matches).toHaveLength(1); expect(matches[0][1]).toBe('/old/path'); @@ -273,7 +296,7 @@ Check out the [old page](/old/path/) for more info. expect(isPlatformLinkLine).toBe(true); // The to= attribute IS matched by the regex... - const jsxLinkRegex = /((?:href|to|url))="(\/[^"#]+?)(?:#[^"]+)?"/g; + const jsxLinkRegex = /((?:href|to|url))="(\/[^"#]+?)(#[^"]+)?"/g; const matches = [...line.matchAll(jsxLinkRegex)]; expect(matches).toHaveLength(1); expect(matches[0][1]).toBe('to'); // attribute name @@ -293,7 +316,7 @@ Check out the [old page](/old/path/) for more info. const isPlatformLinkLine = line.includes('): string { /** * Phase 1: Detect redirect-to-redirect chains + * + * Processes each file's redirects separately to correctly attribute chains + * and handle cases where the same source exists in both files. */ function detectRedirectChains( jsRedirects: {developerDocsRedirects: Redirect[]; userDocsRedirects: Redirect[]}, @@ -120,49 +124,52 @@ function detectRedirectChains( ? mwRedirects.developerDocsRedirects : mwRedirects.userDocsRedirects; - // Build unified map for chain detection + // Build unified map for chain resolution (last entry wins, matching runtime) const allRedirects = [...jsArr, ...mwArr]; const redirectMap = new Map(); for (const r of allRedirects) { redirectMap.set(r.source, r.destination); } - // Track which file each source comes from - const jsSourceSet = new Set(jsArr.map(r => r.source)); - - // Find chains - const seenSources = new Set(); - for (const r of allRedirects) { - if (seenSources.has(r.source)) { - continue; - } - seenSources.add(r.source); - - if (redirectMap.has(r.destination)) { - const chain = walkChain(r.source, redirectMap); - const finalDest = chain[chain.length - 1]; + // Process each file's redirects separately to get correct file attribution. + // Use the unified map for chain walking so cross-file chains are detected. + const fileSources: Array<{file: string; redirects: Redirect[]}> = [ + {file: 'redirects.js', redirects: jsArr}, + {file: 'middleware.ts', redirects: mwArr}, + ]; + + for (const {file, redirects} of fileSources) { + for (const r of redirects) { + // Use the unified map to check if this redirect's destination + // chains into another redirect from either file + if (redirectMap.has(r.destination)) { + const chain = walkChain(r.source, redirectMap); + // Use resolveToFinal for the correct final destination, + // since walkChain caps at maxDepth for display purposes + const finalDest = resolveToFinal(r.source, redirectMap); + + // Skip self-referencing cycles + if (finalDest === r.source || chain.some(s => s.includes('(CYCLE)'))) { + chains.push({ + source: r.source, + currentDest: r.destination, + finalDest: '(CYCLE DETECTED)', + chain, + file, + isDeveloperDocs: isDevDocs, + }); + continue; + } - // Skip self-referencing cycles - if (finalDest === r.source || finalDest.includes('(CYCLE)')) { chains.push({ source: r.source, currentDest: r.destination, - finalDest: '(CYCLE DETECTED)', + finalDest, chain, - file: jsSourceSet.has(r.source) ? 'redirects.js' : 'middleware.ts', + file, isDeveloperDocs: isDevDocs, }); - continue; } - - chains.push({ - source: r.source, - currentDest: r.destination, - finalDest, - chain, - file: jsSourceSet.has(r.source) ? 'redirects.js' : 'middleware.ts', - isDeveloperDocs: isDevDocs, - }); } } } @@ -197,11 +204,11 @@ function detectContentLinkIssues( 'platform-includes', ]); - // Regex patterns for extracting internal links + // Regex patterns for extracting internal links (capturing fragment separately) // Markdown: [text](/path/) or [text](/path/#anchor) - const markdownLinkRegex = /\]\((\/[^)#\s]+?)(?:#[^)]+)?\)/g; + const markdownLinkRegex = /\]\((\/[^)#\s]+?)(#[^)]+)?\)/g; // JSX: href="/path/", to="/path/", or url="/path/" - const jsxLinkRegex = /((?:href|to|url))="(\/[^"#]+?)(?:#[^"]+)?"/g; + const jsxLinkRegex = /((?:href|to|url))="(\/[^"#]+?)(#[^"]+)?"/g; const issues: ContentLinkIssue[] = []; @@ -221,11 +228,12 @@ function detectContentLinkIssues( regex.lastIndex = 0; let match; while ((match = regex.exec(line)) !== null) { - // For jsxLinkRegex, capture group 1 is the attribute name, group 2 is the path - // For markdownLinkRegex, capture group 1 is the path + // For jsxLinkRegex: group 1 = attr name, group 2 = path, group 3 = fragment + // For markdownLinkRegex: group 1 = path, group 2 = fragment const isJsxMatch = regex === jsxLinkRegex; const attrName = isJsxMatch ? match[1] : null; let linkPath = isJsxMatch ? match[2] : match[1]; + const fragment = (isJsxMatch ? match[3] : match[2]) || ''; // Skip PlatformLink to= attributes since PlatformLink prepends // the platform base URL, making them platform-relative paths @@ -248,8 +256,9 @@ function detectContentLinkIssues( issues.push({ filePath, line: i + 1, - linkPath: matchedPath, - finalDest, + linkPath: matchedPath + fragment, + finalDest: finalDest + fragment, + fragment, isDeveloperDocs: isDevDocsFile, }); } From c56fcb2b1da4c9170f620a556a31ef2a4f07af40 Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 15:28:32 -0700 Subject: [PATCH 07/10] fix: walk chain from entry's own destination, not unified map source 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. --- scripts/lint-redirect-chains.spec.ts | 16 ++++++++++++++-- scripts/lint-redirect-chains.ts | 15 +++++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts index 714f52d7d26df..52ff3c87fb64f 100644 --- a/scripts/lint-redirect-chains.spec.ts +++ b/scripts/lint-redirect-chains.spec.ts @@ -140,8 +140,20 @@ describe('detectRedirectChains', () => { // Both entries for /dup/ should be reported with correct file attribution const dupChains = chains.filter(c => c.source === '/dup/'); expect(dupChains).toHaveLength(2); - expect(dupChains.find(c => c.file === 'redirects.js')).toBeDefined(); - expect(dupChains.find(c => c.file === 'middleware.ts')).toBeDefined(); + + const jsChain = dupChains.find(c => c.file === 'redirects.js'); + const mwChain = dupChains.find(c => c.file === 'middleware.ts'); + expect(jsChain).toBeDefined(); + expect(mwChain).toBeDefined(); + + // Each entry's chain should be consistent with its own currentDest + expect(jsChain!.currentDest).toBe('/middle/'); + expect(jsChain!.chain[1]).toBe('/middle/'); // chain starts source -> currentDest + expect(jsChain!.finalDest).toBe('/final/'); + + expect(mwChain!.currentDest).toBe('/other-middle/'); + expect(mwChain!.chain[1]).toBe('/other-middle/'); + expect(mwChain!.finalDest).toBe('/other-final/'); }); it('should return empty array when no chains exist', () => { diff --git a/scripts/lint-redirect-chains.ts b/scripts/lint-redirect-chains.ts index e1607374348ed..4a1fb52fbf39a 100644 --- a/scripts/lint-redirect-chains.ts +++ b/scripts/lint-redirect-chains.ts @@ -140,13 +140,16 @@ function detectRedirectChains( for (const {file, redirects} of fileSources) { for (const r of redirects) { - // Use the unified map to check if this redirect's destination - // chains into another redirect from either file + // Check if this entry's own destination chains into another redirect if (redirectMap.has(r.destination)) { - const chain = walkChain(r.source, redirectMap); - // Use resolveToFinal for the correct final destination, - // since walkChain caps at maxDepth for display purposes - const finalDest = resolveToFinal(r.source, redirectMap); + // Walk from the entry's own destination (not source) to build a + // consistent chain: source -> currentDest -> ... -> finalDest. + // This avoids mismatches when the same source exists in both files + // with different destinations (the unified map would use the other + // file's destination for walkChain(r.source, ...) ). + const tailChain = walkChain(r.destination, redirectMap); + const chain = [r.source, ...tailChain]; + const finalDest = resolveToFinal(r.destination, redirectMap); // Skip self-referencing cycles if (finalDest === r.source || chain.some(s => s.includes('(CYCLE)'))) { From b4ee4f65e25f93dcb1cfc4443675622ce5734a5b Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 15:30:34 -0700 Subject: [PATCH 08/10] fix: paginate listComments to avoid duplicate bot comments 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]'. --- .github/workflows/lint-redirect-chains.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint-redirect-chains.yml b/.github/workflows/lint-redirect-chains.yml index b829b0847c447..c62942722c218 100644 --- a/.github/workflows/lint-redirect-chains.yml +++ b/.github/workflows/lint-redirect-chains.yml @@ -125,8 +125,8 @@ jobs: comment += '---\n'; comment += '*Each redirect hop loses ~15% of SEO link equity and adds latency for users.*\n'; - // Check for existing comments from this action - const {data: comments} = await github.rest.issues.listComments({ + // Check for existing comments from this action (paginate to handle PRs with 30+ comments) + const comments = await github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, @@ -134,7 +134,7 @@ jobs: const existingComment = comments.find(c => c.user.type === 'Bot' && - (c.user.login === 'github-actions[bot]' || c.user.login.includes('bot')) && + c.user.login === 'github-actions[bot]' && c.body.includes('Redirect Chain Issues Detected') ); From 38a09baf90e8ae7792e543026ca754e5ed726e39 Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 15:42:07 -0700 Subject: [PATCH 09/10] fix: apply same pagination fix to check-redirects-on-rename workflow 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. --- .github/workflows/check-redirects-on-rename.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/check-redirects-on-rename.yml b/.github/workflows/check-redirects-on-rename.yml index 3c5f1784cdda0..61ba315002111 100644 --- a/.github/workflows/check-redirects-on-rename.yml +++ b/.github/workflows/check-redirects-on-rename.yml @@ -140,17 +140,17 @@ jobs: comment += '---\n'; comment += '_Note: `middleware.ts` is recommended for simple exact-match redirects. Use `redirects.js` for redirects with path parameters (e.g., `:path*`)._\n'; - // Check for existing comments from this action - const {data: comments} = await github.rest.issues.listComments({ + // Check for existing comments from this action (paginate to handle PRs with 30+ comments) + const comments = await github.paginate(github.rest.issues.listComments, { owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, }); - const existingComment = comments.find(comment => - comment.user.type === 'Bot' && - (comment.user.login === 'github-actions[bot]' || comment.user.login.includes('bot')) && - comment.body.includes('Missing Redirects Detected') + const existingComment = comments.find(c => + c.user.type === 'Bot' && + c.user.login === 'github-actions[bot]' && + c.body.includes('Missing Redirects Detected') ); if (existingComment) { From 42da02027607f5245539a0f903b8f7af67ca6ad0 Mon Sep 17 00:00:00 2001 From: Shannon Anahata Date: Wed, 1 Jul 2026 15:47:12 -0700 Subject: [PATCH 10/10] fix: add trailing slash normalization to Phase 1 chain detection 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. --- scripts/lint-redirect-chains.spec.ts | 19 +++++++++++++++++++ scripts/lint-redirect-chains.ts | 28 ++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/scripts/lint-redirect-chains.spec.ts b/scripts/lint-redirect-chains.spec.ts index 52ff3c87fb64f..6c1362f530fad 100644 --- a/scripts/lint-redirect-chains.spec.ts +++ b/scripts/lint-redirect-chains.spec.ts @@ -224,6 +224,25 @@ describe('detectRedirectChains', () => { expect(chains).toHaveLength(1); expect(chains[0].isDeveloperDocs).toBe(true); }); + + it('should detect chains with trailing slash mismatches', () => { + const jsRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [ + {source: '/old/', destination: '/middle'}, // no trailing slash on dest + {source: '/middle/', destination: '/new/'}, // source has trailing slash + ], + }; + const mwRedirects = { + developerDocsRedirects: [] as Array<{destination: string; source: string}>, + userDocsRedirects: [] as Array<{destination: string; source: string}>, + }; + + const chains = detectRedirectChains(jsRedirects, mwRedirects); + expect(chains).toHaveLength(1); + expect(chains[0].source).toBe('/old/'); + expect(chains[0].finalDest).toBe('/new/'); + }); }); describe('detectContentLinkIssues', () => { diff --git a/scripts/lint-redirect-chains.ts b/scripts/lint-redirect-chains.ts index 4a1fb52fbf39a..5755344dd4160 100644 --- a/scripts/lint-redirect-chains.ts +++ b/scripts/lint-redirect-chains.ts @@ -124,11 +124,17 @@ function detectRedirectChains( ? mwRedirects.developerDocsRedirects : mwRedirects.userDocsRedirects; - // Build unified map for chain resolution (last entry wins, matching runtime) + // Build unified map for chain resolution (last entry wins, matching runtime). + // Store both with and without trailing slash so lookups catch mismatches. const allRedirects = [...jsArr, ...mwArr]; const redirectMap = new Map(); for (const r of allRedirects) { redirectMap.set(r.source, r.destination); + // Also index the normalized form (with trailing slash) if different + const normalized = r.source.endsWith('/') ? r.source : r.source + '/'; + if (normalized !== r.source) { + redirectMap.set(normalized, r.destination); + } } // Process each file's redirects separately to get correct file attribution. @@ -140,16 +146,22 @@ function detectRedirectChains( for (const {file, redirects} of fileSources) { for (const r of redirects) { - // Check if this entry's own destination chains into another redirect - if (redirectMap.has(r.destination)) { + // Check if this entry's destination chains into another redirect, + // normalizing trailing slashes for the lookup + const dest = r.destination; + const destNormalized = dest.endsWith('/') ? dest : dest + '/'; + const matchedDest = redirectMap.has(dest) + ? dest + : redirectMap.has(destNormalized) + ? destNormalized + : null; + + if (matchedDest) { // Walk from the entry's own destination (not source) to build a // consistent chain: source -> currentDest -> ... -> finalDest. - // This avoids mismatches when the same source exists in both files - // with different destinations (the unified map would use the other - // file's destination for walkChain(r.source, ...) ). - const tailChain = walkChain(r.destination, redirectMap); + const tailChain = walkChain(matchedDest, redirectMap); const chain = [r.source, ...tailChain]; - const finalDest = resolveToFinal(r.destination, redirectMap); + const finalDest = resolveToFinal(matchedDest, redirectMap); // Skip self-referencing cycles if (finalDest === r.source || chain.some(s => s.includes('(CYCLE)'))) {