From 3b6cb2d99518ac7c02d383ce5d2b8fe4001ef82d Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:03:41 -0600 Subject: [PATCH 01/29] ci: comment on PRs that break internal links or anchors Add a pull_request workflow that runs dev/check-links.mjs --check-anchors on both the PR head and its merge base, and reports only the findings the PR introduces: outbound links from changed pages, and inbound links from other pages to a page or heading the PR removed or renamed. Pre-existing broken anchors on main are ignored. The job comments on the PR and fails when new breakage is found. dev/check-links.mjs gains --root, --format (text|json|markdown) and --baseline to support that diff, plus case-mismatch detection for routes (links that resolve on macOS but 404 on Linux) and scanning of *.md files. Only *.mdx files count as routes, matching contentlayer's filePathPattern. Amp-Thread-ID: https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261 Co-authored-by: Amp --- .github/workflows/check-links.yml | 88 +++++++++++++++ AGENTS.md | 1 + dev/check-links.mjs | 180 +++++++++++++++++++++++------- 3 files changed, 227 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/check-links.yml diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml new file mode 100644 index 000000000..9ee5eaf46 --- /dev/null +++ b/.github/workflows/check-links.yml @@ -0,0 +1,88 @@ +name: Check links + +# Reports internal links and #anchors that this PR breaks, compared with the +# merge base. Pre-existing broken links on the base branch are ignored. + +on: + pull_request: + +permissions: + contents: read + pull-requests: write + +jobs: + check-links: + name: Broken links introduced by this PR + runs-on: ubuntu-latest + steps: + - name: Check out pull request head + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + + - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: 10.25.0 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: 20.19.6 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Check out merge base + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: git worktree add "$RUNNER_TEMP/base" "$(git merge-base "$BASE_SHA" HEAD)" + + - name: Record broken links already present on the base branch + # Exit 1 means findings, which is expected here + run: | + node dev/check-links.mjs --check-anchors --format json \ + --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-links.json" \ + || [ $? -eq 1 ] + + - name: Find broken links introduced by this PR + id: check + run: | + if node dev/check-links.mjs --check-anchors --format markdown \ + --baseline "$RUNNER_TEMP/base-links.json" > "$RUNNER_TEMP/report.md"; then + echo "broken=false" >> "$GITHUB_OUTPUT" + else + echo "broken=true" >> "$GITHUB_OUTPUT" + fi + cat "$RUNNER_TEMP/report.md" + + - name: Comment on the pull request + # Fork PRs get a read-only token; the report is still in the job log + if: github.event.pull_request.head.repo.full_name == github.repository + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BROKEN: ${{ steps.check.outputs.broken }} + run: | + marker='' + existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ + --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) + + # Only comment when there is something to report, or a stale report to resolve + if [ -z "$existing_comment" ] && [ "$BROKEN" != true ]; then + exit 0 + fi + + { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" + if [ -n "$existing_comment" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \ + --field body=@"$RUNNER_TEMP/comment.md" + else + gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md" + fi + + - name: Fail when this PR introduces broken links + if: steps.check.outputs.broken == 'true' + run: exit 1 diff --git a/AGENTS.md b/AGENTS.md index 9a5e9b001..beafaaf42 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,6 +6,7 @@ - **Build**: `npm run build` - **Dev**: `npm run dev` - **Lint**: `npm run lint` +- **Check links**: `npm run check-links -- --check-anchors` (CI comments on PRs that break links; see `dev/check-links.mjs`) ## AI Chat Integration diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 2a8b24093..6f98fd49c 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -8,10 +8,18 @@ * * Checks for: * - Broken internal links (markdown and JSX/HTML style) + * - Links whose case differs from the real path (work on macOS, 404 on Linux) * - Missing anchor/heading references * - Invalid file paths * - * Usage: node dev/check-links.mjs [--check-anchors] + * Usage: node dev/check-links.mjs [options] + * --check-anchors Also validate #anchors against headings + * --root Repository to check (default: this repository) + * --format Output as text (default), json, or markdown + * --baseline Only report findings absent from this JSON file + * (produced by --format json on another revision) + * + * Exits 1 when any finding is reported. */ import fs from 'fs'; @@ -23,11 +31,23 @@ import { fileURLToPath } from 'url'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); -const DOCS_DIR = path.join(path.dirname(__dirname), 'docs'); - // Parse CLI flags const args = process.argv.slice(2); const CHECK_ANCHORS = args.includes('--check-anchors'); +const ROOT_DIR = path.resolve(flagValue('--root') ?? path.dirname(__dirname)); +const FORMAT = flagValue('--format') ?? 'text'; +const BASELINE_FILE = flagValue('--baseline'); + +const DOCS_DIR = path.join(ROOT_DIR, 'docs'); +// Files whose links are checked. Only .mdx files become site routes; see +// `filePathPattern` in contentlayer.config.ts. +const SOURCE_GLOB = '**/*.{md,mdx}'; +const ROUTE_GLOB = '**/*.mdx'; + +function flagValue(name) { + const index = args.indexOf(name); + return index === -1 ? undefined : args[index + 1]; +} // Regex patterns for extracting links const MARKDOWN_LINK_REGEX = /\[([^\]]*)\]\(([^)]+)\)/g; @@ -56,8 +76,10 @@ function extractHeadings(content) { // Get all MDX files and build a map of valid paths async function buildPathMap() { - const files = await glob('**/*.mdx', { cwd: DOCS_DIR }); + const files = await glob(ROUTE_GLOB, { cwd: DOCS_DIR }); const pathMap = new Map(); + // Lowercased route -> real route, to detect case mismatches + const routesByLowerCase = new Map(); const headingsMap = new Map(); for (const file of files) { @@ -70,6 +92,7 @@ async function buildPathMap() { // Also allow trailing slash variant pathMap.set(routePath, fullPath); pathMap.set(routePath + '/', fullPath); + routesByLowerCase.set(routePath.toLowerCase(), routePath); // Handle index files if (file.endsWith('index.mdx')) { @@ -84,12 +107,12 @@ async function buildPathMap() { headingsMap.set(routePath + '/', headings); } - return { pathMap, headingsMap }; + return { pathMap, routesByLowerCase, headingsMap }; } // Check if a path exists in public directory function checkPublicPath(linkPath) { - const publicPath = path.join(path.dirname(__dirname), 'public', linkPath); + const publicPath = path.join(ROOT_DIR, 'public', linkPath); return fs.existsSync(publicPath); } @@ -137,7 +160,7 @@ function extractLinks(content, filePath) { } // Check if a link is valid -function validateLink(link, currentFile, pathMap, headingsMap) { +function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap }) { const { url } = link; // Skip external links, mailto, tel, javascript, etc. @@ -211,6 +234,14 @@ function validateLink(link, currentFile, pathMap, headingsMap) { return null; } + // Same route with different case: resolves on macOS, 404s on the Linux build + const realRoute = routesByLowerCase.get( + resolvedPath.replace(/\/$/, '').toLowerCase() + ); + if (realRoute) { + return `Case mismatch: "${resolvedPath}" should be "${realRoute}"`; + } + // Check if it's a public asset if (checkPublicPath(resolvedPath)) { return null; @@ -228,60 +259,125 @@ function validateLink(link, currentFile, pathMap, headingsMap) { return `Page not found: "${resolvedPath}"`; } -async function main() { - console.log('πŸ” Checking for dead links in MDX files...\n'); - - const { pathMap, headingsMap } = await buildPathMap(); - const files = await glob('**/*.mdx', { cwd: DOCS_DIR }); +// Find every broken link: [{ file, line, url, error }] +async function findBrokenLinks() { + const maps = await buildPathMap(); + const files = await glob(SOURCE_GLOB, { cwd: DOCS_DIR }); + const findings = []; - let totalErrors = 0; - const errors = []; - - for (const file of files) { + for (const file of files.sort()) { const fullPath = path.join(DOCS_DIR, file); const content = fs.readFileSync(fullPath, 'utf-8'); - const links = extractLinks(content, fullPath); - const fileErrors = []; - - for (const link of links) { - const error = validateLink(link, fullPath, pathMap, headingsMap); + for (const link of extractLinks(content, fullPath)) { + const error = validateLink(link, fullPath, maps); if (error) { - fileErrors.push({ + findings.push({ + file: `docs/${file}`, line: link.lineNumber, url: link.url, error }); } } - - if (fileErrors.length > 0) { - errors.push({ - file: `docs/${file}`, - errors: fileErrors - }); - totalErrors += fileErrors.length; - } } - // Output results - if (errors.length === 0) { - console.log('βœ… No dead links found!'); - process.exit(0); + return findings; +} + +// Identity of a finding across revisions: line numbers shift, so ignore them +function findingKey({ file, url, error }) { + return `${file}\n${url}\n${error}`; +} + +function withoutBaseline(findings, baselineFile) { + const baseline = new Set( + JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey) + ); + return findings.filter(finding => !baseline.has(findingKey(finding))); +} + +function groupByFile(findings) { + const byFile = new Map(); + for (const finding of findings) { + if (!byFile.has(finding.file)) { + byFile.set(finding.file, []); + } + byFile.get(finding.file).push(finding); + } + return byFile; +} + +function formatText(findings) { + const scope = BASELINE_FILE ? 'new ' : ''; + if (findings.length === 0) { + return `βœ… No ${scope}dead links found!\n`; } - console.log(`❌ Found ${totalErrors} dead link(s) in ${errors.length} file(s):\n`); + const byFile = groupByFile(findings); + const lines = [ + `❌ Found ${findings.length} ${scope}dead link(s) in ${byFile.size} file(s):\n` + ]; + for (const [file, fileFindings] of byFile) { + lines.push(`\nπŸ“„ ${file}`); + for (const { line, url, error } of fileFindings) { + lines.push(` Line ${line}: ${url}`); + lines.push(` └─ ${error}`); + } + } + return lines.join('\n') + '\n'; +} + +// Body for a pull request comment +function formatMarkdown(findings) { + if (findings.length === 0) { + return '### βœ… This PR introduces no broken links\n'; + } - for (const { file, errors: fileErrors } of errors) { - console.log(`\nπŸ“„ ${file}`); - for (const { line, url, error } of fileErrors) { - console.log(` Line ${line}: ${url}`); - console.log(` └─ ${error}`); + const lines = [ + `### ❌ This PR introduces ${findings.length} broken link(s)`, + '', + 'Findings in files this PR did not change mean the PR removed or ' + + 'renamed a page or heading that those files link to.', + '' + ]; + for (const [file, fileFindings] of groupByFile(findings)) { + lines.push(`**\`${file}\`**`); + for (const { line, url, error } of fileFindings) { + lines.push(`- line ${line}: \`${url}\` β€” ${error}`); } + lines.push(''); + } + lines.push( + 'Reproduce locally with `pnpm check-links --check-anchors` ' + + '(see `dev/check-links.mjs`).' + ); + return lines.join('\n') + '\n'; +} + +const FORMATTERS = { + text: formatText, + json: findings => JSON.stringify(findings, null, '\t') + '\n', + markdown: formatMarkdown +}; + +async function main() { + const format = FORMATTERS[FORMAT]; + if (!format) { + throw new Error(`Unknown --format "${FORMAT}"; use text, json, or markdown`); } - console.log('\n'); - process.exit(1); + if (FORMAT === 'text') { + console.log('πŸ” Checking for dead links in MDX files...\n'); + } + + let findings = await findBrokenLinks(); + if (BASELINE_FILE) { + findings = withoutBaseline(findings, BASELINE_FILE); + } + + process.stdout.write(format(findings)); + process.exit(findings.length === 0 ? 0 : 1); } main().catch(err => { From fe64d7dd487f7aeb5112c3cb1cb0d3a67faf9276 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:05:39 -0600 Subject: [PATCH 02/29] test: rename a linked heading to exercise the PR comment (will be reverted) Amp-Thread-ID: https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261 Co-authored-by: Amp --- docs/code-search/features.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/code-search/features.mdx b/docs/code-search/features.mdx index b86a4d8f3..fbc1cdedd 100644 --- a/docs/code-search/features.mdx +++ b/docs/code-search/features.mdx @@ -53,7 +53,7 @@ Searching for symbols makes it easier to find specific functions, variables, and Saved searches let you save and describe search queries so you can easily monitor the results on an ongoing basis. You can create a saved search for anything, including diffs and commits across all branches of your repositories. Saved searches can be an early warning system for common problems in your code and a way to monitor best practices, the progress of refactors, etc. -## Search contexts +## Search Contexts (renamed) Search contexts help you search the code you care about on Sourcegraph. A search context represents a set of repositories at specific revisions on a Sourcegraph instance that will be targeted by search queries by default. From 2e1d6b3c339c25410d5786dfcdf7ccebfa8999ed Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:07:01 -0600 Subject: [PATCH 03/29] Revert "test: rename a linked heading to exercise the PR comment (will be reverted)" This reverts commit 1edcff8bf5ccb1cf73d2ec044d1e99c4408019c7. --- docs/code-search/features.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/code-search/features.mdx b/docs/code-search/features.mdx index fbc1cdedd..b86a4d8f3 100644 --- a/docs/code-search/features.mdx +++ b/docs/code-search/features.mdx @@ -53,7 +53,7 @@ Searching for symbols makes it easier to find specific functions, variables, and Saved searches let you save and describe search queries so you can easily monitor the results on an ongoing basis. You can create a saved search for anything, including diffs and commits across all branches of your repositories. Saved searches can be an early warning system for common problems in your code and a way to monitor best practices, the progress of refactors, etc. -## Search Contexts (renamed) +## Search contexts Search contexts help you search the code you care about on Sourcegraph. A search context represents a set of repositories at specific revisions on a Sourcegraph instance that will be targeted by search queries by default. From 386c61871e765515c04f9a282ee0f9e54ba12063 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:13:35 -0600 Subject: [PATCH 04/29] check-links: accept and id= attributes as anchor targets Generated pages such as admin/telemetry/protocol.mdx define anchors with rather than headings. Drops 132 false positives on main (431 -> 299 with --check-anchors). Amp-Thread-ID: https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261 Co-authored-by: Amp --- dev/check-links.mjs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 6f98fd49c..dc2d42f3c 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -54,10 +54,12 @@ const MARKDOWN_LINK_REGEX = /\[([^\]]*)\]\(([^)]+)\)/g; const JSX_HREF_REGEX = /href=["']([^"']+)["']/g; const SRC_ATTR_REGEX = /src=["']([^"']+)["']/g; -// Extract headings from MDX content to build anchor map +// Extract anchor targets from MDX content: heading slugs, plus explicit +// and id="..." attributes function extractHeadings(content) { const slugger = new GithubSlugger(); const headingRegex = /^#{1,6}\s+(.+)$/gm; + const explicitAnchorRegex = /<[a-zA-Z][^>]*\s(?:id|name)=["']([^"']+)["']/g; const headings = new Set(); // Remove code blocks to avoid false positives @@ -71,6 +73,10 @@ function extractHeadings(content) { headings.add(slugger.slug(title.trim())); } + while ((match = explicitAnchorRegex.exec(contentWithoutCode)) !== null) { + headings.add(match[1]); + } + return headings; } From 786135e84930fcd62827952cb7ed2404726fa25c Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:30:04 -0600 Subject: [PATCH 05/29] check-links: detect case mismatches in public/ and docs/ asset links too Replace the fs.existsSync asset checks (case-insensitive on macOS, so they hid links that 404 on the Linux build) with an enumerated lowercase -> real path map of files under public/ and docs/, mirroring the route check. Also register docs/index.mdx as the / route. It was never in the path map (the /index strip needed a leading slash), and existsSync('public/') was masking that by accepting any '/' link. Six pre-existing broken /#anchor links on the homepage are now reported. Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2 Co-authored-by: Amp --- dev/check-links.mjs | 70 +++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index dc2d42f3c..30c462f59 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -80,6 +80,11 @@ function extractHeadings(content) { return headings; } +// Site route for a file under docs/: foo/bar.mdx -> /foo/bar, foo/index.mdx -> /foo, index.mdx -> / +function routeFor(file) { + return '/' + file.replace(/\.mdx$/, '').replace(/(^|\/)index$/, ''); +} + // Get all MDX files and build a map of valid paths async function buildPathMap() { const files = await glob(ROUTE_GLOB, { cwd: DOCS_DIR }); @@ -92,40 +97,35 @@ async function buildPathMap() { const fullPath = path.join(DOCS_DIR, file); const content = fs.readFileSync(fullPath, 'utf-8'); - // Route path (without .mdx extension) - const routePath = '/' + file.replace(/\.mdx$/, '').replace(/\/index$/, ''); + const routePath = routeFor(file); // Also allow trailing slash variant pathMap.set(routePath, fullPath); pathMap.set(routePath + '/', fullPath); routesByLowerCase.set(routePath.toLowerCase(), routePath); - // Handle index files - if (file.endsWith('index.mdx')) { - const dirPath = '/' + file.replace(/\/index\.mdx$/, ''); - pathMap.set(dirPath, fullPath); - pathMap.set(dirPath + '/', fullPath); - } - // Extract headings for anchor validation const headings = extractHeadings(content); headingsMap.set(routePath, headings); headingsMap.set(routePath + '/', headings); } - return { pathMap, routesByLowerCase, headingsMap }; + return { pathMap, routesByLowerCase, headingsMap, assetsByLowerCase: await buildAssetMap() }; } -// Check if a path exists in public directory -function checkPublicPath(linkPath) { - const publicPath = path.join(ROOT_DIR, 'public', linkPath); - return fs.existsSync(publicPath); -} - -// Check if a path exists in docs directory (for images in docs/) -function checkDocsPath(linkPath) { - const docsPath = path.join(DOCS_DIR, linkPath); - return fs.existsSync(docsPath); +// Lowercased link path -> real link path, for files under public/ and docs/ +// (images, PDFs, ...). An enumerated map rather than fs.existsSync, which is +// case-insensitive on macOS and would hide links that 404 on Linux. +async function buildAssetMap() { + const assetsByLowerCase = new Map(); + for (const dir of ['public', 'docs']) { + const files = await glob('**/*', { cwd: path.join(ROOT_DIR, dir), nodir: true }); + for (const file of files) { + const linkPath = '/' + file; + assetsByLowerCase.set(linkPath.toLowerCase(), linkPath); + } + } + return assetsByLowerCase; } // Parse and validate links in a single file @@ -166,7 +166,7 @@ function extractLinks(content, filePath) { } // Check if a link is valid -function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap }) { +function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap, assetsByLowerCase }) { const { url } = link; // Skip external links, mailto, tel, javascript, etc. @@ -193,10 +193,7 @@ function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsM return null; } const anchor = url.substring(1); - const currentRoute = '/' + path.relative(DOCS_DIR, currentFile) - .replace(/\.mdx$/, '') - .replace(/\/index$/, ''); - const headings = headingsMap.get(currentRoute); + const headings = headingsMap.get(routeFor(path.relative(DOCS_DIR, currentFile))); if (headings && !headings.has(anchor)) { return `Anchor "${anchor}" not found in current file`; @@ -240,25 +237,22 @@ function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsM return null; } - // Same route with different case: resolves on macOS, 404s on the Linux build - const realRoute = routesByLowerCase.get( + // Check if it's an asset under public/ or docs/ + const realAsset = assetsByLowerCase.get(resolvedPath.toLowerCase()); + if (realAsset === resolvedPath) { + return null; + } + + // Same route or asset with different case: resolves on macOS, 404s on the Linux build + const realPath = realAsset ?? routesByLowerCase.get( resolvedPath.replace(/\/$/, '').toLowerCase() ); - if (realRoute) { - return `Case mismatch: "${resolvedPath}" should be "${realRoute}"`; - } - - // Check if it's a public asset - if (checkPublicPath(resolvedPath)) { - return null; + if (realPath) { + return `Case mismatch: "${resolvedPath}" should be "${realPath}"`; } // Check if it's a file with extension (like .png, .pdf) if (path.extname(resolvedPath)) { - // Could be an asset - check public folder or docs folder - if (checkPublicPath(resolvedPath) || checkDocsPath(resolvedPath)) { - return null; - } return `File not found: "${resolvedPath}"`; } From d5874ca86e31290ea5e14f2cffd3c4f55108aed6 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:40:59 -0600 Subject: [PATCH 06/29] check-links workflow: never comment on a clean PR A green run posts nothing. If an earlier run left a report and the PR has since been fixed, delete that comment instead of editing it to a checkmark. Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2 Co-authored-by: Amp --- .github/workflows/check-links.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 9ee5eaf46..b849ef247 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -70,8 +70,11 @@ jobs: existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - # Only comment when there is something to report, or a stale report to resolve - if [ -z "$existing_comment" ] && [ "$BROKEN" != true ]; then + # A clean PR carries no comment: remove a stale report once fixed + if [ "$BROKEN" != true ]; then + if [ -n "$existing_comment" ]; then + gh api --method DELETE "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" + fi exit 0 fi From 365e49209c040c11766b55c3132389270955ede4 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 01:43:29 -0600 Subject: [PATCH 07/29] check-links workflow: say what the resolved comment actually means The checkmark comment only ever appears after an earlier run reported breakage, so word it that way instead of "introduces no broken links". Keeps the comment (reverts the delete from the previous commit). Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2 Co-authored-by: Amp --- .github/workflows/check-links.yml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index b849ef247..8c9af1d1b 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -70,15 +70,16 @@ jobs: existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - # A clean PR carries no comment: remove a stale report once fixed - if [ "$BROKEN" != true ]; then - if [ -n "$existing_comment" ]; then - gh api --method DELETE "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" - fi + # Comment only when there is something to report, or an earlier report to resolve + if [ "$BROKEN" = true ]; then + { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" + elif [ -n "$existing_comment" ]; then + printf '%s\n### βœ… The broken links an earlier revision of this PR introduced are fixed\n' \ + "$marker" > "$RUNNER_TEMP/comment.md" + else exit 0 fi - { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" if [ -n "$existing_comment" ]; then gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \ --field body=@"$RUNNER_TEMP/comment.md" From 8fda8532015d8bab7005053891d5740a3db413bc Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Sun, 6 Sep 2026 04:12:25 -0600 Subject: [PATCH 08/29] check-links: strip fences line by line, first file wins a route, same-page anchors use own headings Amp-Thread-ID: https://ampcode.com/threads/T-01a07623-9d65-7356-96b8-2bebb31ffa5a Co-authored-by: Amp --- dev/check-links.mjs | 58 +++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 30c462f59..196c202a0 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -54,6 +54,36 @@ const MARKDOWN_LINK_REGEX = /\[([^\]]*)\]\(([^)]+)\)/g; const JSX_HREF_REGEX = /href=["']([^"']+)["']/g; const SRC_ATTR_REGEX = /src=["']([^"']+)["']/g; +// A fence opener/closer is a run of 3+ backticks or tildes at the start of a line. +const FENCE_LINE_REGEX = /^\s*(`{3,}|~{3,})/; + +// Blank out fenced code blocks, keeping line numbers intact, so `# comment` +// lines and example links inside them are ignored. Walks line by line: a naive +// /```[\s\S]*?```/ regex also matches inline backtick runs in prose (e.g. +// `"true```), which flips every later fence pairing. +function stripFencedCodeBlocks(content) { + let openFence; + return content.split('\n').map(line => { + const fence = line.match(FENCE_LINE_REGEX)?.[1]; + if (openFence) { + const closesOpenFence = + fence !== undefined && + fence[0] === openFence[0] && + fence.length >= openFence.length && + line.trim() === fence; + if (closesOpenFence) { + openFence = undefined; + } + return ''; + } + if (fence) { + openFence = fence; + return ''; + } + return line; + }).join('\n'); +} + // Extract anchor targets from MDX content: heading slugs, plus explicit // and id="..." attributes function extractHeadings(content) { @@ -62,8 +92,7 @@ function extractHeadings(content) { const explicitAnchorRegex = /<[a-zA-Z][^>]*\s(?:id|name)=["']([^"']+)["']/g; const headings = new Set(); - // Remove code blocks to avoid false positives - const contentWithoutCode = content.replace(/```[\s\S]*?```/g, ''); + const contentWithoutCode = stripFencedCodeBlocks(content); let match; while ((match = headingRegex.exec(contentWithoutCode)) !== null) { @@ -87,30 +116,33 @@ function routeFor(file) { // Get all MDX files and build a map of valid paths async function buildPathMap() { - const files = await glob(ROUTE_GLOB, { cwd: DOCS_DIR }); + // Sorted so foo.mdx precedes foo/index.mdx; when both exist the site serves + // the first match (allPosts.find), so the first file owns the route here too. + const files = (await glob(ROUTE_GLOB, { cwd: DOCS_DIR })).sort(); const pathMap = new Map(); // Lowercased route -> real route, to detect case mismatches const routesByLowerCase = new Map(); const headingsMap = new Map(); + // Absolute file path -> headings, for same-page #anchor links + const headingsByFile = new Map(); for (const file of files) { const fullPath = path.join(DOCS_DIR, file); - const content = fs.readFileSync(fullPath, 'utf-8'); + const headings = extractHeadings(fs.readFileSync(fullPath, 'utf-8')); + headingsByFile.set(fullPath, headings); const routePath = routeFor(file); + if (pathMap.has(routePath)) continue; // Also allow trailing slash variant pathMap.set(routePath, fullPath); pathMap.set(routePath + '/', fullPath); routesByLowerCase.set(routePath.toLowerCase(), routePath); - - // Extract headings for anchor validation - const headings = extractHeadings(content); headingsMap.set(routePath, headings); headingsMap.set(routePath + '/', headings); } - return { pathMap, routesByLowerCase, headingsMap, assetsByLowerCase: await buildAssetMap() }; + return { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase: await buildAssetMap() }; } // Lowercased link path -> real link path, for files under public/ and docs/ @@ -132,11 +164,7 @@ async function buildAssetMap() { function extractLinks(content, filePath) { const links = []; - // Remove code blocks to avoid checking links in code examples - const contentWithoutCode = content.replace(/```[\s\S]*?```/g, (match) => { - // Replace with same number of newlines to preserve line numbers - return match.replace(/[^\n]/g, ' '); - }); + const contentWithoutCode = stripFencedCodeBlocks(content); // Extract markdown links [text](url) let match; @@ -166,7 +194,7 @@ function extractLinks(content, filePath) { } // Check if a link is valid -function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap, assetsByLowerCase }) { +function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase }) { const { url } = link; // Skip external links, mailto, tel, javascript, etc. @@ -193,7 +221,7 @@ function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsM return null; } const anchor = url.substring(1); - const headings = headingsMap.get(routeFor(path.relative(DOCS_DIR, currentFile))); + const headings = headingsByFile.get(currentFile); if (headings && !headings.has(anchor)) { return `Anchor "${anchor}" not found in current file`; From 0ac55b2a6100ade0c9a0950aefb8da1c3540864c Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:44:23 -0600 Subject: [PATCH 09/29] Add verify-links-live: prove a branch's changed links resolve on a deployed site Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a07623-9d65-7356-96b8-2bebb31ffa5a --- AGENTS.md | 1 + dev/check-links.mjs | 15 +++-- dev/verify-links-live.mjs | 130 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 dev/verify-links-live.mjs diff --git a/AGENTS.md b/AGENTS.md index beafaaf42..cdb3ac2a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,6 +7,7 @@ - **Dev**: `npm run dev` - **Lint**: `npm run lint` - **Check links**: `npm run check-links -- --check-anchors` (CI comments on PRs that break links; see `dev/check-links.mjs`) +- **Prove changed links resolve on a deploy**: `node dev/verify-links-live.mjs --site ` prints a Markdown table for the PR description ## AI Chat Integration diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 196c202a0..df4cd878a 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -26,7 +26,7 @@ import fs from 'fs'; import path from 'path'; import { glob } from 'glob'; import GithubSlugger from 'github-slugger'; -import { fileURLToPath } from 'url'; +import { fileURLToPath, pathToFileURL } from 'url'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -86,7 +86,7 @@ function stripFencedCodeBlocks(content) { // Extract anchor targets from MDX content: heading slugs, plus explicit // and id="..." attributes -function extractHeadings(content) { +export function extractHeadings(content) { const slugger = new GithubSlugger(); const headingRegex = /^#{1,6}\s+(.+)$/gm; const explicitAnchorRegex = /<[a-zA-Z][^>]*\s(?:id|name)=["']([^"']+)["']/g; @@ -408,7 +408,10 @@ async function main() { process.exit(findings.length === 0 ? 0 : 1); } -main().catch(err => { - console.error('Error running link checker:', err); - process.exit(1); -}); +// Only run when executed directly; dev/verify-links-live.mjs imports extractHeadings. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch(err => { + console.error('Error running link checker:', err); + process.exit(1); + }); +} diff --git a/dev/verify-links-live.mjs b/dev/verify-links-live.mjs new file mode 100644 index 000000000..7f4434a3e --- /dev/null +++ b/dev/verify-links-live.mjs @@ -0,0 +1,130 @@ +#!/usr/bin/env node +// Prove that the links a branch changed resolve on a deployed site. +// +// For every internal link that differs between a base ref and the working +// tree, fetch the target page on --site and check the rendered HTML, not the +// HTTP status: the docs site serves its not-found page with 200 (see the +// dynamicParams fix) and even real pages embed the not-found text in their +// RSC payload, so neither status nor page text proves anything. A link passes +// when +// - the target page's own first heading id (from its local MDX source) is an +// id in the HTML, which the not-found page never has, and +// - the link's #fragment, when present, is an id in the HTML. +// Prints a Markdown table to paste into a PR. Old links point at --old-site so +// reviewers can see the current breakage. +// +// node dev/verify-links-live.mjs --site https://.vercel.app [--old-site https://sourcegraph.com/docs] [--base origin/main] +// +// Production serves under https://sourcegraph.com/docs (basePath in +// next.config.js); Vercel previews serve at the root, so pass the full prefix +// in --site. +import { execSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { extractHeadings } from './check-links.mjs'; + +const args = process.argv.slice(2); +const argValue = (flag, fallback) => { + const index = args.indexOf(flag); + return index === -1 ? fallback : args[index + 1]; +}; +const SITE = argValue('--site', 'https://sourcegraph.com/docs').replace(/\/$/, ''); +const OLD_SITE = argValue('--old-site', 'https://sourcegraph.com/docs').replace(/\/$/, ''); +const BASE_REF = argValue('--base', 'origin/main'); + +const LINK = /\]\(([^)\s]+)\)|href=["']([^"']+)["']/g; + +function routeFor(file) { + return '/' + file.replace(/^docs\//, '').replace(/\.mdx$/, '').replace(/\/index$/, ''); +} + +// Same precedence as the site (first glob match): foo.mdx before foo/index.mdx. +function sourceFileFor(route) { + const stem = route === '/' ? 'docs/index' : `docs${route}`; + return [`${stem}.mdx`, `${stem}/index.mdx`].find(candidate => fs.existsSync(candidate)); +} + +const firstHeadingCache = new Map(); +function firstHeadingId(route) { + if (!firstHeadingCache.has(route)) { + const file = sourceFileFor(route); + const [first] = file ? extractHeadings(fs.readFileSync(file, 'utf-8')) : []; + firstHeadingCache.set(route, first); + } + return firstHeadingCache.get(route); +} + +// Collect { file, oldUrl, newUrl } for every link that changed. +function changedLinks() { + const diff = execSync(`git diff -U0 ${BASE_REF}`, { encoding: 'utf-8' }); + const result = []; + let file, removed = [], added = []; + const flush = () => { + if (removed.length === added.length) removed.forEach((oldLine, index) => { + const oldLinks = [...oldLine.matchAll(LINK)].map(m => m[1] ?? m[2]); + const newLinks = [...added[index].matchAll(LINK)].map(m => m[1] ?? m[2]); + if (oldLinks.length !== newLinks.length) return; + oldLinks.forEach((oldUrl, i) => { + if (oldUrl !== newLinks[i]) result.push({ file, oldUrl, newUrl: newLinks[i] }); + }); + }); + removed = []; added = []; + }; + for (const line of diff.split('\n')) { + if (line.startsWith('+++ b/')) { flush(); file = line.slice(6); continue; } + if (line.startsWith('@@')) { flush(); continue; } + if (line.startsWith('---')) continue; + if (line.startsWith('-')) removed.push(line.slice(1)); + else if (line.startsWith('+')) added.push(line.slice(1)); + } + flush(); + return result; +} + +function resolveTarget(file, url) { + if (/^(https?:|mailto:|tel:)/.test(url)) return null; + const [pagePart, fragment] = url.split('#'); + let page; + if (pagePart === '') page = routeFor(file); + else if (pagePart.startsWith('/')) page = pagePart; + else page = path.posix.join(path.posix.dirname(routeFor(file)), pagePart); + page = page.replace(/\/$/, '') || '/'; + return { page, fragment, href: `${page}${fragment ? '#' + fragment : ''}` }; +} + +const pageCache = new Map(); +async function fetchPage(page) { + if (!pageCache.has(page)) { + pageCache.set(page, fetch(`${SITE}${page}`, { redirect: 'follow' }).then(response => response.text())); + } + return pageCache.get(page); +} + +function hasId(html, id) { + const escaped = id.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`\\sid=["']${escaped}["']`).test(html); +} + +const links = changedLinks(); +const rows = []; +let failures = 0; +for (const { file, oldUrl, newUrl } of links) { + const target = resolveTarget(file, newUrl); + if (!target) continue; + const html = await fetchPage(target.page); + const heading = firstHeadingId(target.page); + // No local source file means no such page; a page with no headings cannot be verified either. + const pageOk = Boolean(heading) && hasId(html, heading); + const anchorOk = !target.fragment || hasId(html, target.fragment); + if (!pageOk || !anchorOk) failures++; + const oldTarget = resolveTarget(file, oldUrl); + const oldCell = oldTarget ? `[\`${oldUrl}\`](${OLD_SITE}${oldTarget.href})` : `\`${oldUrl}\``; + rows.push(`| \`${file}\` | ${oldCell} | [\`${newUrl}\`](${SITE}${target.href}) | ${pageOk ? 'βœ…' : '❌'} | ${target.fragment ? (anchorOk ? 'βœ…' : '❌') : 'β€”'} |`); +} + +console.log(`Checked ${rows.length} changed links against ${SITE}: ${rows.length - failures} resolve, ${failures} fail.`); +console.log('Page rendered = the target page\'s first heading id is present (the 404 page never has it); Anchor = the #fragment is an id on the page. Old links point at the current site.\n'); +console.log('| File containing the link | Old link (broken today) | New link (preview) | Page rendered | Anchor found |'); +console.log('|--------------------------|-------------------------|--------------------|---------------|--------------|'); +console.log(rows.join('\n')); +process.exitCode = failures ? 1 : 0; From eb4c2480dfcc6c234f425c73b622c79879bc79d6 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:36:32 -0600 Subject: [PATCH 10/29] check-links: slug the full heading text when a heading contains a link, matching rehype-slug Amp-Thread-ID: https://ampcode.com/threads/T-01a07623-9d65-7356-96b8-2bebb31ffa5a Co-authored-by: Amp --- dev/check-links.mjs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index df4cd878a..07ad7b0bd 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -96,9 +96,9 @@ export function extractHeadings(content) { let match; while ((match = headingRegex.exec(contentWithoutCode)) !== null) { - // Handle headings with links: [Text](/path) -> Text - const linkMatch = match[1].match(/\[([^\]]+)\]\([^)]+\)/); - const title = linkMatch ? linkMatch[1] : match[1]; + // rehype-slug slugs the heading's full text, with links reduced to their text: + // "How can I use [GitHub expression syntax](url) literally" -> "How can I use GitHub expression syntax literally" + const title = match[1].replace(/\[([^\]]+)\]\([^)]+\)/g, '$1'); headings.add(slugger.slug(title.trim())); } From a3fa71fa67c0b8dc7c166f9a193c45e091142210 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 04:46:48 -0600 Subject: [PATCH 11/29] check-links: state that redirects do not satisfy the PR check The checker resolves links against docs/**/*.mdx routes only and never reads src/data/redirects.ts, so a redirect added alongside a page move still leaves inbound links reported. Say so in the PR comment and in AGENTS.md so contributors update the links instead. Amp-Thread-ID: https://ampcode.com/threads/T-01a085bf-fe07-77a1-a4e2-55bff4679bcc Co-authored-by: Amp --- AGENTS.md | 2 +- dev/check-links.mjs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index cdb3ac2a2..1af2ac832 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,7 +6,7 @@ - **Build**: `npm run build` - **Dev**: `npm run dev` - **Lint**: `npm run lint` -- **Check links**: `npm run check-links -- --check-anchors` (CI comments on PRs that break links; see `dev/check-links.mjs`) +- **Check links**: `npm run check-links -- --check-anchors` (CI comments on PRs that break links; see `dev/check-links.mjs`). When moving a page or renaming a heading, update every link to it; a redirect in `src/data/redirects.ts` does not satisfy the check - **Prove changed links resolve on a deploy**: `node dev/verify-links-live.mjs --site ` prints a Markdown table for the PR description ## AI Chat Integration diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 07ad7b0bd..3bb3cceab 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -366,7 +366,9 @@ function formatMarkdown(findings) { `### ❌ This PR introduces ${findings.length} broken link(s)`, '', 'Findings in files this PR did not change mean the PR removed or ' + - 'renamed a page or heading that those files link to.', + 'renamed a page or heading that those files link to. Update those ' + + 'links: adding a redirect in `src/data/redirects.ts` does not ' + + 'satisfy this check.', '' ]; for (const [file, fileFindings] of groupByFile(findings)) { From aa980246d7006867e2f07f6c6662c954b803cc7a Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:20:42 -0600 Subject: [PATCH 12/29] check-links: reword the inbound-link guidance in the PR comment Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-links.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 3bb3cceab..7203b75b1 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -365,10 +365,10 @@ function formatMarkdown(findings) { const lines = [ `### ❌ This PR introduces ${findings.length} broken link(s)`, '', - 'Findings in files this PR did not change mean the PR removed or ' + - 'renamed a page or heading that those files link to. Update those ' + - 'links: adding a redirect in `src/data/redirects.ts` does not ' + - 'satisfy this check.', + 'Any new broken links found on pages not changed in this PR indicate ' + + 'your PR has broken inbound links. Please fix the inbound links on ' + + 'the other pages. Adding a redirect in `src/data/redirects.ts` does ' + + 'not satisfy this check.', '' ]; for (const [file, fileFindings] of groupByFile(findings)) { From 6780dac7a6b517d01fe4d63fa85932488d0854c6 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:22:44 -0600 Subject: [PATCH 13/29] check-links: split the inbound-link guidance and say why redirects do not count Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-links.mjs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 7203b75b1..5dbe9fe28 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -365,10 +365,12 @@ function formatMarkdown(findings) { const lines = [ `### ❌ This PR introduces ${findings.length} broken link(s)`, '', - 'Any new broken links found on pages not changed in this PR indicate ' + + 'Any broken links found here on pages not changed in this PR indicate ' + 'your PR has broken inbound links. Please fix the inbound links on ' + - 'the other pages. Adding a redirect in `src/data/redirects.ts` does ' + - 'not satisfy this check.', + 'the other pages.', + '', + 'Adding a redirect in `src/data/redirects.ts` does not satisfy this ' + + 'check, because it’s a workaround instead of a fix.', '' ]; for (const [file, fileFindings] of groupByFile(findings)) { From 5531c042f258967134d3e7384f1c4784dbc45256 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:24:49 -0600 Subject: [PATCH 14/29] check-links: link each file path in the PR comment to the file on the PR branch Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- .github/workflows/check-links.yml | 6 +++++- dev/check-links.mjs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 8c9af1d1b..472f677df 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -49,9 +49,13 @@ jobs: - name: Find broken links introduced by this PR id: check + env: + # File links in the report open the file on the PR branch + LINK_BASE: ${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }} run: | if node dev/check-links.mjs --check-anchors --format markdown \ - --baseline "$RUNNER_TEMP/base-links.json" > "$RUNNER_TEMP/report.md"; then + --baseline "$RUNNER_TEMP/base-links.json" \ + --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then echo "broken=false" >> "$GITHUB_OUTPUT" else echo "broken=true" >> "$GITHUB_OUTPUT" diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 5dbe9fe28..3cef6facc 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -18,6 +18,8 @@ * --format Output as text (default), json, or markdown * --baseline Only report findings absent from this JSON file * (produced by --format json on another revision) + * --link-base Markdown output links each file path to /, + * e.g. https://github.com/sourcegraph/docs/blob/ * * Exits 1 when any finding is reported. */ @@ -37,6 +39,7 @@ const CHECK_ANCHORS = args.includes('--check-anchors'); const ROOT_DIR = path.resolve(flagValue('--root') ?? path.dirname(__dirname)); const FORMAT = flagValue('--format') ?? 'text'; const BASELINE_FILE = flagValue('--baseline'); +const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); const DOCS_DIR = path.join(ROOT_DIR, 'docs'); // Files whose links are checked. Only .mdx files become site routes; see @@ -374,7 +377,8 @@ function formatMarkdown(findings) { '' ]; for (const [file, fileFindings] of groupByFile(findings)) { - lines.push(`**\`${file}\`**`); + const fileLabel = `**\`${file}\`**`; + lines.push(LINK_BASE ? `[${fileLabel}](${LINK_BASE}/${file})` : fileLabel); for (const { line, url, error } of fileFindings) { lines.push(`- line ${line}: \`${url}\` β€” ${error}`); } From b6bdbcb6e5c149c792bcf5529a0ff6e26fd128d5 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:38:57 -0600 Subject: [PATCH 15/29] check-links: install only github-slugger in CI, link line numbers to the source view Drop glob in favour of fs.readdirSync(recursive) so the PR check needs one package instead of the whole site, and skip the pnpm/setup-node steps. Every finding now links to the file and line on the PR branch, in the ?plain=1 code view where #L anchors work. Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- .github/workflows/check-links.yml | 20 ++++------- dev/check-links.mjs | 58 ++++++++++++++++++------------- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 472f677df..ee3868760 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -21,19 +21,13 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10.25.0 - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - node-version: 20.19.6 - cache: pnpm - - - name: Install dependencies - run: pnpm install --frozen-lockfile + - name: Install github-slugger, the only dependency of dev/check-links.mjs + # Into a scratch prefix, not the repo: `npm install ` next to + # package.json would install every dependency of the site + run: | + npm install --prefix "$RUNNER_TEMP/deps" --no-package-lock --no-audit --no-fund \ + "github-slugger@$(node -p 'require("./package.json").dependencies["github-slugger"]')" + ln -s "$RUNNER_TEMP/deps/node_modules" node_modules - name: Check out merge base env: diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 3cef6facc..690d38a78 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -26,7 +26,6 @@ import fs from 'fs'; import path from 'path'; -import { glob } from 'glob'; import GithubSlugger from 'github-slugger'; import { fileURLToPath, pathToFileURL } from 'url'; @@ -44,14 +43,26 @@ const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); const DOCS_DIR = path.join(ROOT_DIR, 'docs'); // Files whose links are checked. Only .mdx files become site routes; see // `filePathPattern` in contentlayer.config.ts. -const SOURCE_GLOB = '**/*.{md,mdx}'; -const ROUTE_GLOB = '**/*.mdx'; +const SOURCE_EXTENSIONS = ['.md', '.mdx']; +const ROUTE_EXTENSIONS = ['.mdx']; function flagValue(name) { const index = args.indexOf(name); return index === -1 ? undefined : args[index + 1]; } +// Sorted relative paths of every file under dir, optionally limited to some +// extensions. Sorted so foo.mdx precedes foo/index.mdx; when both exist the +// site serves the first match (allPosts.find), so the first file owns the route. +function listFiles(dir, extensions) { + if (!fs.existsSync(dir)) return []; + return fs + .readdirSync(dir, { recursive: true, withFileTypes: true }) + .filter(entry => entry.isFile() && (!extensions || extensions.includes(path.extname(entry.name)))) + .map(entry => path.relative(dir, path.join(entry.parentPath, entry.name))) + .sort(); +} + // Regex patterns for extracting links const MARKDOWN_LINK_REGEX = /\[([^\]]*)\]\(([^)]+)\)/g; const JSX_HREF_REGEX = /href=["']([^"']+)["']/g; @@ -118,10 +129,8 @@ function routeFor(file) { } // Get all MDX files and build a map of valid paths -async function buildPathMap() { - // Sorted so foo.mdx precedes foo/index.mdx; when both exist the site serves - // the first match (allPosts.find), so the first file owns the route here too. - const files = (await glob(ROUTE_GLOB, { cwd: DOCS_DIR })).sort(); +function buildPathMap() { + const files = listFiles(DOCS_DIR, ROUTE_EXTENSIONS); const pathMap = new Map(); // Lowercased route -> real route, to detect case mismatches const routesByLowerCase = new Map(); @@ -145,17 +154,16 @@ async function buildPathMap() { headingsMap.set(routePath + '/', headings); } - return { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase: await buildAssetMap() }; + return { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase: buildAssetMap() }; } // Lowercased link path -> real link path, for files under public/ and docs/ // (images, PDFs, ...). An enumerated map rather than fs.existsSync, which is // case-insensitive on macOS and would hide links that 404 on Linux. -async function buildAssetMap() { +function buildAssetMap() { const assetsByLowerCase = new Map(); for (const dir of ['public', 'docs']) { - const files = await glob('**/*', { cwd: path.join(ROOT_DIR, dir), nodir: true }); - for (const file of files) { + for (const file of listFiles(path.join(ROOT_DIR, dir))) { const linkPath = '/' + file; assetsByLowerCase.set(linkPath.toLowerCase(), linkPath); } @@ -291,12 +299,11 @@ function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsM } // Find every broken link: [{ file, line, url, error }] -async function findBrokenLinks() { - const maps = await buildPathMap(); - const files = await glob(SOURCE_GLOB, { cwd: DOCS_DIR }); +function findBrokenLinks() { + const maps = buildPathMap(); const findings = []; - for (const file of files.sort()) { + for (const file of listFiles(DOCS_DIR, SOURCE_EXTENSIONS)) { const fullPath = path.join(DOCS_DIR, file); const content = fs.readFileSync(fullPath, 'utf-8'); @@ -359,6 +366,10 @@ function formatText(findings) { return lines.join('\n') + '\n'; } +function linkTo(text, url) { + return url ? `[${text}](${url})` : text; +} + // Body for a pull request comment function formatMarkdown(findings) { if (findings.length === 0) { @@ -377,10 +388,12 @@ function formatMarkdown(findings) { '' ]; for (const [file, fileFindings] of groupByFile(findings)) { - const fileLabel = `**\`${file}\`**`; - lines.push(LINK_BASE ? `[${fileLabel}](${LINK_BASE}/${file})` : fileLabel); + // ?plain=1 opens GitHub's code view, where #L anchors work; the rendered + // Markdown preview ignores them + const fileUrl = LINK_BASE && `${LINK_BASE}/${file}?plain=1`; + lines.push(linkTo(`**\`${file}\`**`, fileUrl)); for (const { line, url, error } of fileFindings) { - lines.push(`- line ${line}: \`${url}\` β€” ${error}`); + lines.push(`- ${linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`)}: \`${url}\` β€” ${error}`); } lines.push(''); } @@ -397,7 +410,7 @@ const FORMATTERS = { markdown: formatMarkdown }; -async function main() { +function main() { const format = FORMATTERS[FORMAT]; if (!format) { throw new Error(`Unknown --format "${FORMAT}"; use text, json, or markdown`); @@ -407,7 +420,7 @@ async function main() { console.log('πŸ” Checking for dead links in MDX files...\n'); } - let findings = await findBrokenLinks(); + let findings = findBrokenLinks(); if (BASELINE_FILE) { findings = withoutBaseline(findings, BASELINE_FILE); } @@ -418,8 +431,5 @@ async function main() { // Only run when executed directly; dev/verify-links-live.mjs imports extractHeadings. if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - main().catch(err => { - console.error('Error running link checker:', err); - process.exit(1); - }); + main(); } From 7e827bc546987f29c1da35a37ce2e1f78c6fb4e9 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:00:58 -0600 Subject: [PATCH 16/29] check-links: split the PR comment into outbound and inbound broken links Outbound findings are in files the PR changed (it added or edited a bad link); inbound ones are elsewhere (the PR renamed or removed a link target). The workflow passes git diff --name-only against the merge base. --- .github/workflows/check-links.yml | 6 ++- dev/check-links.mjs | 71 +++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index ee3868760..821704258 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -32,7 +32,10 @@ jobs: - name: Check out merge base env: BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: git worktree add "$RUNNER_TEMP/base" "$(git merge-base "$BASE_SHA" HEAD)" + run: | + merge_base=$(git merge-base "$BASE_SHA" HEAD) + git worktree add "$RUNNER_TEMP/base" "$merge_base" + git diff --name-only "$merge_base" HEAD > "$RUNNER_TEMP/changed-files.txt" - name: Record broken links already present on the base branch # Exit 1 means findings, which is expected here @@ -49,6 +52,7 @@ jobs: run: | if node dev/check-links.mjs --check-anchors --format markdown \ --baseline "$RUNNER_TEMP/base-links.json" \ + --changed-files "$RUNNER_TEMP/changed-files.txt" \ --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then echo "broken=false" >> "$GITHUB_OUTPUT" else diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 690d38a78..02dfbdbe9 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -20,6 +20,8 @@ * (produced by --format json on another revision) * --link-base Markdown output links each file path to /, * e.g. https://github.com/sourcegraph/docs/blob/ + * --changed-files Markdown output splits findings into outbound (in one + * of these files, one path per line) and inbound (elsewhere) * * Exits 1 when any finding is reported. */ @@ -39,6 +41,13 @@ const ROOT_DIR = path.resolve(flagValue('--root') ?? path.dirname(__dirname)); const FORMAT = flagValue('--format') ?? 'text'; const BASELINE_FILE = flagValue('--baseline'); const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); +const CHANGED_FILES = readPathList(flagValue('--changed-files')); + +// Set of the non-empty lines of file, or undefined when no file is given +function readPathList(file) { + if (!file) return undefined; + return new Set(fs.readFileSync(file, 'utf-8').split('\n').filter(Boolean)); +} const DOCS_DIR = path.join(ROOT_DIR, 'docs'); // Files whose links are checked. Only .mdx files become site routes; see @@ -370,23 +379,9 @@ function linkTo(text, url) { return url ? `[${text}](${url})` : text; } -// Body for a pull request comment -function formatMarkdown(findings) { - if (findings.length === 0) { - return '### βœ… This PR introduces no broken links\n'; - } - - const lines = [ - `### ❌ This PR introduces ${findings.length} broken link(s)`, - '', - 'Any broken links found here on pages not changed in this PR indicate ' + - 'your PR has broken inbound links. Please fix the inbound links on ' + - 'the other pages.', - '', - 'Adding a redirect in `src/data/redirects.ts` does not satisfy this ' + - 'check, because it’s a workaround instead of a fix.', - '' - ]; +// Markdown list of findings grouped by file, linked to the source when --link-base is set +function markdownFindingList(findings) { + const lines = []; for (const [file, fileFindings] of groupByFile(findings)) { // ?plain=1 opens GitHub's code view, where #L anchors work; the rendered // Markdown preview ignores them @@ -397,9 +392,49 @@ function formatMarkdown(findings) { } lines.push(''); } + return lines; +} + +// Body for a pull request comment. With --changed-files, findings are split into +// outbound (in a file this PR changed: the PR added or edited a bad link) and +// inbound (in a file it did not: the PR renamed or removed a link target). +function formatMarkdown(findings) { + if (findings.length === 0) { + return '### βœ… This PR introduces no broken links\n'; + } + + const lines = [`### ❌ This PR introduces ${findings.length} broken link(s)`, '']; + if (CHANGED_FILES) { + const outbound = findings.filter(finding => CHANGED_FILES.has(finding.file)); + const inbound = findings.filter(finding => !CHANGED_FILES.has(finding.file)); + if (outbound.length > 0) { + lines.push( + '### Outbound', + '', + 'Your PR includes links to pages or anchors that do not exist.', + '', + ...markdownFindingList(outbound) + ); + } + if (inbound.length > 0) { + lines.push( + '### Inbound', + '', + 'A change your PR made broke inbound links from elsewhere. ' + + 'Please fix the inbound links on the other pages.', + '', + ...markdownFindingList(inbound) + ); + } + } else { + lines.push(...markdownFindingList(findings)); + } lines.push( 'Reproduce locally with `pnpm check-links --check-anchors` ' + - '(see `dev/check-links.mjs`).' + '(see `dev/check-links.mjs`).', + '', + 'Adding a redirect in `src/data/redirects.ts` does not satisfy this ' + + 'check, because it’s a workaround instead of a fix.' ); return lines.join('\n') + '\n'; } From c6d0c2256eed11641b30b4e97a0b91009ef55785 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:22:23 -0600 Subject: [PATCH 17/29] check-links: export listFiles and routeFor for check-redirects, cancel superseded runs Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- .github/workflows/check-links.yml | 5 +++++ dev/check-links.mjs | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 821704258..f2c5b5132 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -6,6 +6,11 @@ name: Check links on: pull_request: +# A new push supersedes the run for the previous one +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + permissions: contents: read pull-requests: write diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 02dfbdbe9..18f36c42b 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -63,7 +63,7 @@ function flagValue(name) { // Sorted relative paths of every file under dir, optionally limited to some // extensions. Sorted so foo.mdx precedes foo/index.mdx; when both exist the // site serves the first match (allPosts.find), so the first file owns the route. -function listFiles(dir, extensions) { +export function listFiles(dir, extensions) { if (!fs.existsSync(dir)) return []; return fs .readdirSync(dir, { recursive: true, withFileTypes: true }) @@ -133,7 +133,7 @@ export function extractHeadings(content) { } // Site route for a file under docs/: foo/bar.mdx -> /foo/bar, foo/index.mdx -> /foo, index.mdx -> / -function routeFor(file) { +export function routeFor(file) { return '/' + file.replace(/\.mdx$/, '').replace(/(^|\/)index$/, ''); } @@ -464,7 +464,8 @@ function main() { process.exit(findings.length === 0 ? 0 : 1); } -// Only run when executed directly; dev/verify-links-live.mjs imports extractHeadings. +// Only run when executed directly; dev/verify-links-live.mjs and dev/check-redirects.mjs +// import the exported helpers. if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main(); } From 506485c3a528250f9858aa291f3738e057604b27 Mon Sep 17 00:00:00 2001 From: Marc <7050295+marcleblanc2@users.noreply.github.com> Date: Tue, 8 Sep 2026 01:47:47 +0000 Subject: [PATCH 18/29] ci: fail PRs that break redirects Add dev/check-redirects.mjs, which validates every entry in src/data/redirects.ts: the destination page exists under docs/ (following chains through other redirects), a #fragment destination names a heading that exists on that page, and the source does not shadow an existing page. Add a Check redirects workflow that runs the script on the PR head and on the merge base and fails only on findings the PR introduces, so the 227 redirects already broken on main do not block unrelated PRs. It comments the report on the PR and updates that comment on later pushes, matching the Check links workflow. Co-authored-by: Amp Amp-Thread-ID: https://ampcode.com/threads/T-01a07e6f-db41-74af-bbeb-f8952e637289 --- .github/workflows/check-redirects.yml | 94 ++++++++++ dev/check-redirects.mjs | 250 ++++++++++++++++++++++++++ package.json | 1 + 3 files changed, 345 insertions(+) create mode 100644 .github/workflows/check-redirects.yml create mode 100644 dev/check-redirects.mjs diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml new file mode 100644 index 000000000..492150641 --- /dev/null +++ b/.github/workflows/check-redirects.yml @@ -0,0 +1,94 @@ +name: Check redirects + +# Reports redirects in src/data/redirects.ts that this PR breaks, compared with +# the merge base: destinations that no longer exist, #fragments whose heading +# was renamed, and new redirects that shadow an existing page. Pre-existing +# broken redirects on the base branch are ignored. + +on: + pull_request: + +permissions: + contents: read + pull-requests: write + +jobs: + check-redirects: + name: Broken redirects introduced by this PR + runs-on: ubuntu-latest + steps: + - name: Check out pull request head + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + + - name: Install pnpm + uses: pnpm/action-setup@v4 + with: + version: 10.25.0 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: 20.19.6 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Check out merge base + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: git worktree add "$RUNNER_TEMP/base" "$(git merge-base "$BASE_SHA" HEAD)" + + - name: Record broken redirects already present on the base branch + # Exit 1 means findings, which is expected here + run: | + node dev/check-redirects.mjs --format json \ + --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-redirects.json" \ + || [ $? -eq 1 ] + + - name: Find broken redirects introduced by this PR + id: check + run: | + if node dev/check-redirects.mjs --format markdown \ + --baseline "$RUNNER_TEMP/base-redirects.json" > "$RUNNER_TEMP/report.md"; then + echo "broken=false" >> "$GITHUB_OUTPUT" + else + echo "broken=true" >> "$GITHUB_OUTPUT" + fi + cat "$RUNNER_TEMP/report.md" + + - name: Comment on the pull request + # Fork PRs get a read-only token; the report is still in the job log + if: github.event.pull_request.head.repo.full_name == github.repository + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BROKEN: ${{ steps.check.outputs.broken }} + run: | + marker='' + existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ + --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) + + # Comment only when there is something to report, or an earlier report to resolve + if [ "$BROKEN" = true ]; then + { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" + elif [ -n "$existing_comment" ]; then + printf '%s\n### βœ… The broken redirects an earlier revision of this PR introduced are fixed\n' \ + "$marker" > "$RUNNER_TEMP/comment.md" + else + exit 0 + fi + + if [ -n "$existing_comment" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \ + --field body=@"$RUNNER_TEMP/comment.md" + else + gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md" + fi + + - name: Fail when this PR introduces broken redirects + if: steps.check.outputs.broken == 'true' + run: exit 1 diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs new file mode 100644 index 000000000..71f0d2fd9 --- /dev/null +++ b/dev/check-redirects.mjs @@ -0,0 +1,250 @@ +#!/usr/bin/env node + +/** + * Broken redirect checker for src/data/redirects.ts. + * + * A redirect is broken when a visitor who follows it does not end up on a real + * page. Checks, for every entry: + * - the destination page exists under docs/ (or is a file under public/), + * following chains through other redirects + * - when the destination has a #fragment, the heading exists on that page + * - the source does not shadow an existing page (the middleware would redirect + * visitors away from a page that exists) + * + * External (http) destinations are not checked. + * + * Usage: node dev/check-redirects.mjs [--format text|json|markdown] + * [--root ] [--baseline ] + * + * --root Check a different checkout (for example the merge base). + * --baseline Ignore findings also present in this JSON report, so only + * redirects broken by the current change are reported. + * + * Exit code 1 when there are findings, 0 otherwise. + */ + +import fs from 'fs'; +import path from 'path'; +import vm from 'vm'; +import {glob} from 'glob'; +import GithubSlugger from 'github-slugger'; +import {fileURLToPath} from 'url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +const args = process.argv.slice(2); +function flag(name, fallback) { + const i = args.indexOf(name); + return i === -1 ? fallback : args[i + 1]; +} +const ROOT = path.resolve(flag('--root', path.dirname(__dirname))); +const FORMAT = flag('--format', 'text'); +const BASELINE = flag('--baseline'); + +const REDIRECTS_FILE = path.join(ROOT, 'src/data/redirects.ts'); +const CONSTANTS_FILE = path.join(ROOT, 'src/data/constants.ts'); +const DOCS_DIR = path.join(ROOT, 'docs'); +const PUBLIC_DIR = path.join(ROOT, 'public'); +const MAX_CHAIN = 10; + +// Load redirects.ts without a TypeScript toolchain. The file is plain data +// plus one import, so strip the module syntax and evaluate it. +function loadRedirects() { + const source = fs.readFileSync(REDIRECTS_FILE, 'utf-8'); + const constants = fs.readFileSync(CONSTANTS_FILE, 'utf-8'); + const rss = constants.match( + /TECHNICAL_CHANGELOG_RSS_URL\s*=\s*['"]([^'"]+)['"]/ + ); + + const script = source + .replace(/^import .*$/gm, '') + .replace(/^export const /gm, 'const ') + .replace(/module\.exports\s*=\s*\{[\s\S]*?\};?/g, ''); + + const sandbox = {TECHNICAL_CHANGELOG_RSS_URL: rss ? rss[1] : ''}; + vm.runInNewContext(`${script}\nresult = updatedRedirectsData;`, sandbox); + + // Line number of each entry, for the report. Entries are written one + // `source:` per line; if that assumption fails, omit line numbers. + const lines = source.split('\n'); + const arrayEnd = lines.findIndex(line => /^\];?\s*$/.test(line)); + const sourceLines = []; + lines.slice(0, arrayEnd).forEach((line, i) => { + if (/^\s*source:/.test(line)) sourceLines.push(i + 1); + }); + const haveLines = sourceLines.length === sandbox.result.length; + + return sandbox.result.map((r, i) => ({ + source: r.source, + destination: r.destination, + line: haveLines ? sourceLines[i] : undefined + })); +} + +// Same heading extraction as dev/check-links.mjs, so both checks agree on +// which anchors exist. +function extractHeadings(content) { + const slugger = new GithubSlugger(); + const headings = new Set(); + const contentWithoutCode = content.replace(/```[\s\S]*?```/g, ''); + const headingRegex = /^#{1,6}\s+(.+)$/gm; + let match; + while ((match = headingRegex.exec(contentWithoutCode)) !== null) { + const linkMatch = match[1].match(/\[([^\]]+)\]\([^)]+\)/); + const title = linkMatch ? linkMatch[1] : match[1]; + headings.add(slugger.slug(title.trim())); + } + return headings; +} + +async function buildRoutes() { + const files = await glob('**/*.mdx', {cwd: DOCS_DIR}); + const headingsByRoute = new Map(); + for (const file of files) { + const route = + '/' + file.replace(/\.mdx$/, '').replace(/(^|\/)index$/, ''); + const content = fs.readFileSync(path.join(DOCS_DIR, file), 'utf-8'); + headingsByRoute.set( + route === '' ? '/' : route, + extractHeadings(content) + ); + } + return headingsByRoute; +} + +function split(url) { + const [pathAndQuery, hash = ''] = url.split('#'); + const p = pathAndQuery.split('?')[0].replace(/\/+$/, '') || '/'; + return {path: p, hash: decodeURIComponent(hash)}; +} + +function isPublicFile(p) { + const full = path.join(PUBLIC_DIR, p); + return full.startsWith(PUBLIC_DIR) && fs.existsSync(full); +} + +function check(redirects, headingsByRoute) { + const findings = []; + const firstBySource = new Map(); + for (const r of redirects) { + if (!firstBySource.has(r.source)) firstBySource.set(r.source, r); + } + const report = (r, problem) => + findings.push({ + source: r.source, + destination: r.destination, + line: r.line, + problem + }); + + for (const r of redirects) { + // Only the first entry for a source is reachable; later duplicates are + // dead code and cannot break anything. + if (firstBySource.get(r.source) !== r) continue; + + const src = split(r.source); + if (!src.hash && headingsByRoute.has(src.path)) { + report( + r, + `source is an existing page; visitors to it are redirected away` + ); + } + + if (/^https?:\/\//.test(r.destination)) continue; + + // Follow redirect chains to the page a visitor finally lands on. + let dest = split(r.destination); + let hops = 0; + const seen = new Set([r.source]); + while ( + firstBySource.has(dest.path) && + !headingsByRoute.has(dest.path) + ) { + if (seen.has(dest.path) || ++hops > MAX_CHAIN) { + report( + r, + `redirect loop or chain longer than ${MAX_CHAIN} hops` + ); + dest = null; + break; + } + seen.add(dest.path); + const next = firstBySource.get(dest.path).destination; + if (/^https?:\/\//.test(next)) { + dest = null; + break; + } + const nextSplit = split(next); + // A hop without its own fragment keeps the fragment we have. + dest = {path: nextSplit.path, hash: nextSplit.hash || dest.hash}; + } + if (!dest) continue; + + const headings = headingsByRoute.get(dest.path); + if (!headings) { + if (!isPublicFile(dest.path)) { + report(r, `destination page ${dest.path} does not exist`); + } + continue; + } + if (dest.hash && !headings.has(dest.hash)) { + report(r, `heading #${dest.hash} not found on ${dest.path}`); + } + } + return findings; +} + +const key = f => `${f.source}\u0000${f.destination}\u0000${f.problem}`; + +function applyBaseline(findings) { + if (!BASELINE) return findings; + const baseline = new Set( + JSON.parse(fs.readFileSync(BASELINE, 'utf-8')).map(key) + ); + return findings.filter(f => !baseline.has(key(f))); +} + +function print(findings) { + if (FORMAT === 'json') { + console.log(JSON.stringify(findings, null, 2)); + return; + } + const scope = BASELINE ? 'broken by this PR' : 'broken'; + if (FORMAT === 'markdown') { + if (findings.length === 0) { + console.log(`### βœ… No redirects ${scope}`); + return; + } + console.log( + `### ❌ ${findings.length} redirect${findings.length === 1 ? '' : 's'} ${scope}\n` + ); + console.log( + 'Visitors following these redirects do not land on a real page. Fix the destination in `src/data/redirects.ts`, or add a redirect for a page this PR removed.\n' + ); + console.log('| Line | Source | Destination | Problem |'); + console.log('| --- | --- | --- | --- |'); + for (const f of findings) { + console.log( + `| ${f.line ?? ''} | \`${f.source}\` | \`${f.destination}\` | ${f.problem} |` + ); + } + return; + } + if (findings.length === 0) { + console.log(`βœ… No redirects ${scope}`); + return; + } + console.log(`❌ ${findings.length} redirect(s) ${scope}:\n`); + for (const f of findings) { + const where = f.line + ? `src/data/redirects.ts:${f.line}` + : 'src/data/redirects.ts'; + console.log( + ` ${where}\n ${f.source} -> ${f.destination}\n ${f.problem}\n` + ); + } +} + +const findings = applyBaseline(check(loadRedirects(), await buildRoutes())); +print(findings); +process.exit(findings.length > 0 ? 1 : 0); diff --git a/package.json b/package.json index 93900c805..4b0d0af99 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "check-links": "node dev/check-links.mjs", "check-filenames": "node dev/check-filenames.mjs", "check-images": "node dev/check-images.mjs", + "check-redirects": "node dev/check-redirects.mjs", "generate-mermaid-logos": "node dev/generate-aws-icons.mjs", "baseai": "baseai", "sync": "npx baseai@latest deploy -m memory-sg-docs-live", From 38c3748f319569d7ef884b8af71cf6143b6eefa9 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:26:33 -0600 Subject: [PATCH 19/29] check-redirects: share heading extraction with check-links, install only github-slugger, link report lines, rename to Redirect check Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- .github/workflows/check-redirects.yml | 45 ++-- dev/check-redirects.mjs | 329 +++++++++++++------------- 2 files changed, 193 insertions(+), 181 deletions(-) diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml index 492150641..b601a68e2 100644 --- a/.github/workflows/check-redirects.yml +++ b/.github/workflows/check-redirects.yml @@ -1,4 +1,4 @@ -name: Check redirects +name: Redirect check # Reports redirects in src/data/redirects.ts that this PR breaks, compared with # the merge base: destinations that no longer exist, #fragments whose heading @@ -8,13 +8,18 @@ name: Check redirects on: pull_request: +# A new push supersedes the run for the previous one +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + permissions: contents: read pull-requests: write jobs: - check-redirects: - name: Broken redirects introduced by this PR + redirect-check: + name: Redirect check runs-on: ubuntu-latest steps: - name: Check out pull request head @@ -23,19 +28,13 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - - name: Install pnpm - uses: pnpm/action-setup@v4 - with: - version: 10.25.0 - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - node-version: 20.19.6 - cache: pnpm - - - name: Install dependencies - run: pnpm install --frozen-lockfile + - name: Install github-slugger, the only dependency of dev/check-redirects.mjs + # Into a scratch prefix, not the repo: `npm install ` next to + # package.json would install every dependency of the site + run: | + npm install --prefix "$RUNNER_TEMP/deps" --no-package-lock --no-audit --no-fund \ + "github-slugger@$(node -p 'require("./package.json").dependencies["github-slugger"]')" + ln -s "$RUNNER_TEMP/deps/node_modules" node_modules - name: Check out merge base env: @@ -49,11 +48,15 @@ jobs: --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-redirects.json" \ || [ $? -eq 1 ] - - name: Find broken redirects introduced by this PR + - name: Find redirects broken by this PR id: check + env: + # Line links in the report open redirects.ts on the PR branch + LINK_BASE: ${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }} run: | if node dev/check-redirects.mjs --format markdown \ - --baseline "$RUNNER_TEMP/base-redirects.json" > "$RUNNER_TEMP/report.md"; then + --baseline "$RUNNER_TEMP/base-redirects.json" \ + --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then echo "broken=false" >> "$GITHUB_OUTPUT" else echo "broken=true" >> "$GITHUB_OUTPUT" @@ -68,7 +71,7 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} BROKEN: ${{ steps.check.outputs.broken }} run: | - marker='' + marker='' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) @@ -76,7 +79,7 @@ jobs: if [ "$BROKEN" = true ]; then { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" elif [ -n "$existing_comment" ]; then - printf '%s\n### βœ… The broken redirects an earlier revision of this PR introduced are fixed\n' \ + printf '%s\n### βœ… The redirects an earlier revision of this PR broke are fixed\n' \ "$marker" > "$RUNNER_TEMP/comment.md" else exit 0 @@ -89,6 +92,6 @@ jobs: gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md" fi - - name: Fail when this PR introduces broken redirects + - name: Fail when this PR breaks redirects if: steps.check.outputs.broken == 'true' run: exit 1 diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 71f0d2fd9..bc8d81b48 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -1,10 +1,10 @@ #!/usr/bin/env node /** - * Broken redirect checker for src/data/redirects.ts. + * Redirect check for src/data/redirects.ts. * - * A redirect is broken when a visitor who follows it does not end up on a real - * page. Checks, for every entry: + * Redirects exist so external traffic to an old URL still reaches a page, so + * each one must be correct. Checks, for every entry: * - the destination page exists under docs/ (or is a file under public/), * following chains through other redirects * - when the destination has a #fragment, the heading exists on that page @@ -13,55 +13,58 @@ * * External (http) destinations are not checked. * - * Usage: node dev/check-redirects.mjs [--format text|json|markdown] - * [--root ] [--baseline ] + * Usage: node dev/check-redirects.mjs [options] + * --root Repository to check (default: this repository) + * --format Output as text (default), json, or markdown + * --baseline Only report findings absent from this JSON file + * (produced by --format json on another revision) + * --link-base Markdown output links each line to + * /src/data/redirects.ts, e.g. + * https://github.com/sourcegraph/docs/blob/ * - * --root Check a different checkout (for example the merge base). - * --baseline Ignore findings also present in this JSON report, so only - * redirects broken by the current change are reported. - * - * Exit code 1 when there are findings, 0 otherwise. + * Exits 1 when any finding is reported. */ import fs from 'fs'; import path from 'path'; import vm from 'vm'; -import {glob} from 'glob'; -import GithubSlugger from 'github-slugger'; -import {fileURLToPath} from 'url'; +import { fileURLToPath } from 'url'; +import { extractHeadings, listFiles, routeFor } from './check-links.mjs'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const args = process.argv.slice(2); -function flag(name, fallback) { - const i = args.indexOf(name); - return i === -1 ? fallback : args[i + 1]; -} -const ROOT = path.resolve(flag('--root', path.dirname(__dirname))); -const FORMAT = flag('--format', 'text'); -const BASELINE = flag('--baseline'); +const ROOT_DIR = path.resolve(flagValue('--root') ?? path.dirname(__dirname)); +const FORMAT = flagValue('--format') ?? 'text'; +const BASELINE_FILE = flagValue('--baseline'); +const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); -const REDIRECTS_FILE = path.join(ROOT, 'src/data/redirects.ts'); -const CONSTANTS_FILE = path.join(ROOT, 'src/data/constants.ts'); -const DOCS_DIR = path.join(ROOT, 'docs'); -const PUBLIC_DIR = path.join(ROOT, 'public'); -const MAX_CHAIN = 10; +const REDIRECTS_PATH = 'src/data/redirects.ts'; +const REDIRECTS_FILE = path.join(ROOT_DIR, REDIRECTS_PATH); +const CONSTANTS_FILE = path.join(ROOT_DIR, 'src/data/constants.ts'); +const DOCS_DIR = path.join(ROOT_DIR, 'docs'); +const PUBLIC_DIR = path.join(ROOT_DIR, 'public'); +const MAX_CHAIN_HOPS = 10; + +function flagValue(name) { + const index = args.indexOf(name); + return index === -1 ? undefined : args[index + 1]; +} // Load redirects.ts without a TypeScript toolchain. The file is plain data // plus one import, so strip the module syntax and evaluate it. +// Returns [{ source, destination, line }]. function loadRedirects() { const source = fs.readFileSync(REDIRECTS_FILE, 'utf-8'); const constants = fs.readFileSync(CONSTANTS_FILE, 'utf-8'); - const rss = constants.match( - /TECHNICAL_CHANGELOG_RSS_URL\s*=\s*['"]([^'"]+)['"]/ - ); + const rssUrl = constants.match(/TECHNICAL_CHANGELOG_RSS_URL\s*=\s*['"]([^'"]+)['"]/)?.[1] ?? ''; const script = source .replace(/^import .*$/gm, '') .replace(/^export const /gm, 'const ') .replace(/module\.exports\s*=\s*\{[\s\S]*?\};?/g, ''); - const sandbox = {TECHNICAL_CHANGELOG_RSS_URL: rss ? rss[1] : ''}; + const sandbox = { TECHNICAL_CHANGELOG_RSS_URL: rssUrl }; vm.runInNewContext(`${script}\nresult = updatedRedirectsData;`, sandbox); // Line number of each entry, for the report. Entries are written one @@ -69,182 +72,188 @@ function loadRedirects() { const lines = source.split('\n'); const arrayEnd = lines.findIndex(line => /^\];?\s*$/.test(line)); const sourceLines = []; - lines.slice(0, arrayEnd).forEach((line, i) => { - if (/^\s*source:/.test(line)) sourceLines.push(i + 1); + lines.slice(0, arrayEnd).forEach((line, index) => { + if (/^\s*source:/.test(line)) sourceLines.push(index + 1); }); const haveLines = sourceLines.length === sandbox.result.length; - return sandbox.result.map((r, i) => ({ - source: r.source, - destination: r.destination, - line: haveLines ? sourceLines[i] : undefined + return sandbox.result.map((redirect, index) => ({ + source: redirect.source, + destination: redirect.destination, + line: haveLines ? sourceLines[index] : undefined })); } -// Same heading extraction as dev/check-links.mjs, so both checks agree on -// which anchors exist. -function extractHeadings(content) { - const slugger = new GithubSlugger(); - const headings = new Set(); - const contentWithoutCode = content.replace(/```[\s\S]*?```/g, ''); - const headingRegex = /^#{1,6}\s+(.+)$/gm; - let match; - while ((match = headingRegex.exec(contentWithoutCode)) !== null) { - const linkMatch = match[1].match(/\[([^\]]+)\]\([^)]+\)/); - const title = linkMatch ? linkMatch[1] : match[1]; - headings.add(slugger.slug(title.trim())); - } - return headings; -} - -async function buildRoutes() { - const files = await glob('**/*.mdx', {cwd: DOCS_DIR}); +// Site route -> Set of anchors on that page. When foo.mdx and foo/index.mdx +// both exist the first (sorted) file owns the route, as in check-links.mjs. +function buildHeadingsByRoute() { const headingsByRoute = new Map(); - for (const file of files) { - const route = - '/' + file.replace(/\.mdx$/, '').replace(/(^|\/)index$/, ''); - const content = fs.readFileSync(path.join(DOCS_DIR, file), 'utf-8'); - headingsByRoute.set( - route === '' ? '/' : route, - extractHeadings(content) - ); + for (const file of listFiles(DOCS_DIR, ['.mdx'])) { + const route = routeFor(file); + if (headingsByRoute.has(route)) continue; + headingsByRoute.set(route, extractHeadings(fs.readFileSync(path.join(DOCS_DIR, file), 'utf-8'))); } return headingsByRoute; } -function split(url) { - const [pathAndQuery, hash = ''] = url.split('#'); - const p = pathAndQuery.split('?')[0].replace(/\/+$/, '') || '/'; - return {path: p, hash: decodeURIComponent(hash)}; +// '/foo/bar/?x=1#baz' -> { pathname: '/foo/bar', fragment: 'baz' } +function splitUrl(url) { + const [pathAndQuery, fragment = ''] = url.split('#'); + const pathname = pathAndQuery.split('?')[0].replace(/\/+$/, '') || '/'; + return { pathname, fragment: decodeURIComponent(fragment) }; } -function isPublicFile(p) { - const full = path.join(PUBLIC_DIR, p); - return full.startsWith(PUBLIC_DIR) && fs.existsSync(full); +function isPublicFile(pathname) { + const fullPath = path.join(PUBLIC_DIR, pathname); + return fullPath.startsWith(PUBLIC_DIR) && fs.existsSync(fullPath); } -function check(redirects, headingsByRoute) { +function isExternal(url) { + return /^https?:\/\//.test(url); +} + +// Every incorrect redirect: [{ source, destination, line, problem }] +function findBrokenRedirects(redirects, headingsByRoute) { const findings = []; + // Only the first entry for a source is reachable; later duplicates are + // dead code and cannot break anything. const firstBySource = new Map(); - for (const r of redirects) { - if (!firstBySource.has(r.source)) firstBySource.set(r.source, r); + for (const redirect of redirects) { + if (!firstBySource.has(redirect.source)) firstBySource.set(redirect.source, redirect); } - const report = (r, problem) => - findings.push({ - source: r.source, - destination: r.destination, - line: r.line, - problem - }); - - for (const r of redirects) { - // Only the first entry for a source is reachable; later duplicates are - // dead code and cannot break anything. - if (firstBySource.get(r.source) !== r) continue; - - const src = split(r.source); - if (!src.hash && headingsByRoute.has(src.path)) { - report( - r, - `source is an existing page; visitors to it are redirected away` - ); + const report = (redirect, problem) => findings.push({ ...redirect, problem }); + + for (const redirect of redirects) { + if (firstBySource.get(redirect.source) !== redirect) continue; + + const source = splitUrl(redirect.source); + if (!source.fragment && headingsByRoute.has(source.pathname)) { + report(redirect, 'source is an existing page; visitors to it are redirected away'); } - if (/^https?:\/\//.test(r.destination)) continue; + if (isExternal(redirect.destination)) continue; - // Follow redirect chains to the page a visitor finally lands on. - let dest = split(r.destination); + // Follow redirect chains to the page a visitor finally lands on + let destination = splitUrl(redirect.destination); let hops = 0; - const seen = new Set([r.source]); - while ( - firstBySource.has(dest.path) && - !headingsByRoute.has(dest.path) - ) { - if (seen.has(dest.path) || ++hops > MAX_CHAIN) { - report( - r, - `redirect loop or chain longer than ${MAX_CHAIN} hops` - ); - dest = null; + const visited = new Set([redirect.source]); + while (firstBySource.has(destination.pathname) && !headingsByRoute.has(destination.pathname)) { + if (visited.has(destination.pathname) || ++hops > MAX_CHAIN_HOPS) { + report(redirect, `redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops`); + destination = undefined; break; } - seen.add(dest.path); - const next = firstBySource.get(dest.path).destination; - if (/^https?:\/\//.test(next)) { - dest = null; + visited.add(destination.pathname); + const next = firstBySource.get(destination.pathname).destination; + if (isExternal(next)) { + destination = undefined; break; } - const nextSplit = split(next); - // A hop without its own fragment keeps the fragment we have. - dest = {path: nextSplit.path, hash: nextSplit.hash || dest.hash}; + const nextUrl = splitUrl(next); + // A hop without its own fragment keeps the fragment we have + destination = { pathname: nextUrl.pathname, fragment: nextUrl.fragment || destination.fragment }; } - if (!dest) continue; + if (!destination) continue; - const headings = headingsByRoute.get(dest.path); + const headings = headingsByRoute.get(destination.pathname); if (!headings) { - if (!isPublicFile(dest.path)) { - report(r, `destination page ${dest.path} does not exist`); + if (!isPublicFile(destination.pathname)) { + report(redirect, `destination page ${destination.pathname} does not exist`); } continue; } - if (dest.hash && !headings.has(dest.hash)) { - report(r, `heading #${dest.hash} not found on ${dest.path}`); + if (destination.fragment && !headings.has(destination.fragment)) { + report(redirect, `heading #${destination.fragment} not found on ${destination.pathname}`); } } return findings; } -const key = f => `${f.source}\u0000${f.destination}\u0000${f.problem}`; +// Line numbers are left out so an entry that only moved is not a new finding +function findingKey(finding) { + return `${finding.source}\u0000${finding.destination}\u0000${finding.problem}`; +} -function applyBaseline(findings) { - if (!BASELINE) return findings; - const baseline = new Set( - JSON.parse(fs.readFileSync(BASELINE, 'utf-8')).map(key) - ); - return findings.filter(f => !baseline.has(key(f))); +function withoutBaseline(findings, baselineFile) { + const baseline = new Set(JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey)); + return findings.filter(finding => !baseline.has(findingKey(finding))); } -function print(findings) { - if (FORMAT === 'json') { - console.log(JSON.stringify(findings, null, 2)); - return; +function formatText(findings) { + const scope = BASELINE_FILE ? 'broken by this change' : 'broken'; + if (findings.length === 0) { + return `βœ… No redirects ${scope}\n`; } - const scope = BASELINE ? 'broken by this PR' : 'broken'; - if (FORMAT === 'markdown') { - if (findings.length === 0) { - console.log(`### βœ… No redirects ${scope}`); - return; - } - console.log( - `### ❌ ${findings.length} redirect${findings.length === 1 ? '' : 's'} ${scope}\n` + const lines = [`❌ ${findings.length} redirect(s) ${scope}:`, '']; + for (const { source, destination, line, problem } of findings) { + lines.push( + ` ${REDIRECTS_PATH}${line ? `:${line}` : ''}`, + ` ${source} -> ${destination}`, + ` ${problem}`, + '' ); - console.log( - 'Visitors following these redirects do not land on a real page. Fix the destination in `src/data/redirects.ts`, or add a redirect for a page this PR removed.\n' - ); - console.log('| Line | Source | Destination | Problem |'); - console.log('| --- | --- | --- | --- |'); - for (const f of findings) { - console.log( - `| ${f.line ?? ''} | \`${f.source}\` | \`${f.destination}\` | ${f.problem} |` - ); - } - return; } + return lines.join('\n'); +} + +function linkTo(text, url) { + return url ? `[${text}](${url})` : text; +} + +// Body for a pull request comment +function formatMarkdown(findings) { if (findings.length === 0) { - console.log(`βœ… No redirects ${scope}`); - return; + return '### βœ… This PR breaks no redirects\n'; } - console.log(`❌ ${findings.length} redirect(s) ${scope}:\n`); - for (const f of findings) { - const where = f.line - ? `src/data/redirects.ts:${f.line}` - : 'src/data/redirects.ts'; - console.log( - ` ${where}\n ${f.source} -> ${f.destination}\n ${f.problem}\n` - ); + + // ?plain=1 opens GitHub's code view, where #L anchors work + const fileUrl = LINK_BASE && `${LINK_BASE}/${REDIRECTS_PATH}?plain=1`; + const lines = [ + `### ❌ This PR breaks ${findings.length} redirect(s)`, + '', + 'Redirects exist so external traffic (search results, bookmarks) to an old URL ' + + 'still reaches a page. Each one must point at a page and heading that exist, ' + + 'and must not shadow a page that exists.', + '', + '- If this PR moved or renamed the redirect destination, then update the ' + + 'redirect with the updated destination', + '', + '- If this PR removed the destination, then either update the destination to ' + + 'the next most relevant page, or remove it to leave the user with our 404 page', + '', + 'Redirects are not to be used for internal links (tech debt snowball), internal ' + + 'links must be fixed; the "Check links" comment lists any this PR broke.', + '', + '| Line | Source | Destination | Problem |', + '| --- | --- | --- | --- |' + ]; + for (const { source, destination, line, problem } of findings) { + const lineCell = line ? linkTo(line, fileUrl && `${fileUrl}#L${line}`) : ''; + lines.push(`| ${lineCell} | \`${source}\` | \`${destination}\` | ${problem} |`); } + lines.push('', 'Reproduce locally with `pnpm check-redirects` (see `dev/check-redirects.mjs`).'); + return lines.join('\n') + '\n'; +} + +const FORMATTERS = { + text: formatText, + json: findings => JSON.stringify(findings, null, '\t') + '\n', + markdown: formatMarkdown +}; + +function main() { + const format = FORMATTERS[FORMAT]; + if (!format) { + throw new Error(`Unknown --format "${FORMAT}"; use text, json, or markdown`); + } + + let findings = findBrokenRedirects(loadRedirects(), buildHeadingsByRoute()); + if (BASELINE_FILE) { + findings = withoutBaseline(findings, BASELINE_FILE); + } + + process.stdout.write(format(findings)); + process.exit(findings.length === 0 ? 0 : 1); } -const findings = applyBaseline(check(loadRedirects(), await buildRoutes())); -print(findings); -process.exit(findings.length > 0 ? 1 : 0); +main(); From dd1cc11239fe043667f331d3837be5ee2c3c358c Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:32:07 -0600 Subject: [PATCH 20/29] check-redirects: rename to "Check redirects / Broken redirects introduced by this PR" Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- .github/workflows/check-redirects.yml | 8 ++++---- dev/check-redirects.mjs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml index b601a68e2..c3f0e24b2 100644 --- a/.github/workflows/check-redirects.yml +++ b/.github/workflows/check-redirects.yml @@ -1,4 +1,4 @@ -name: Redirect check +name: Check redirects # Reports redirects in src/data/redirects.ts that this PR breaks, compared with # the merge base: destinations that no longer exist, #fragments whose heading @@ -18,8 +18,8 @@ permissions: pull-requests: write jobs: - redirect-check: - name: Redirect check + check-redirects: + name: Broken redirects introduced by this PR runs-on: ubuntu-latest steps: - name: Check out pull request head @@ -71,7 +71,7 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} BROKEN: ${{ steps.check.outputs.broken }} run: | - marker='' + marker='' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index bc8d81b48..d4cb12b15 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node /** - * Redirect check for src/data/redirects.ts. + * Checks redirects in src/data/redirects.ts. * * Redirects exist so external traffic to an old URL still reaches a page, so * each one must be correct. Checks, for every entry: From a4c33ce16586b78c312bfbd3fc8b11d0ea91f86d Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:44:13 -0600 Subject: [PATCH 21/29] check-redirects: list broken entries as they appear in redirects.ts instead of a table Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index d4cb12b15..e2c271cb8 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -224,12 +224,18 @@ function formatMarkdown(findings) { 'Redirects are not to be used for internal links (tech debt snowball), internal ' + 'links must be fixed; the "Check links" comment lists any this PR broke.', '', - '| Line | Source | Destination | Problem |', - '| --- | --- | --- | --- |' + linkTo(`**\`${REDIRECTS_PATH}\`**`, fileUrl) ]; + // Each entry is shown as it appears in the redirects file, so it is easy to find there for (const { source, destination, line, problem } of findings) { - const lineCell = line ? linkTo(line, fileUrl && `${fileUrl}#L${line}`) : ''; - lines.push(`| ${lineCell} | \`${source}\` | \`${destination}\` | ${problem} |`); + const where = line ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) : 'entry'; + lines.push( + `- ${where}: ${problem}`, + ' ```ts', + ` source: '${source}',`, + ` destination: '${destination}'`, + ' ```' + ); } lines.push('', 'Reproduce locally with `pnpm check-redirects` (see `dev/check-redirects.mjs`).'); return lines.join('\n') + '\n'; From d3f3ef26802081f93e2b5585fd48fe6d4fa70f39 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:08:26 -0600 Subject: [PATCH 22/29] check-redirects: group findings by problem, flag #fragment sources that can never match, new comment text Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 146 +++++++++++++++++++++++++++++----------- 1 file changed, 108 insertions(+), 38 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index e2c271cb8..7d57cdb76 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -10,6 +10,8 @@ * - when the destination has a #fragment, the heading exists on that page * - the source does not shadow an existing page (the middleware would redirect * visitors away from a page that exists) + * - the source has no #fragment: browsers never send fragments, so such an + * entry can never match * * External (http) destinations are not checked. * @@ -28,8 +30,8 @@ import fs from 'fs'; import path from 'path'; import vm from 'vm'; -import { fileURLToPath } from 'url'; -import { extractHeadings, listFiles, routeFor } from './check-links.mjs'; +import {fileURLToPath} from 'url'; +import {extractHeadings, listFiles, routeFor} from './check-links.mjs'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -57,14 +59,17 @@ function flagValue(name) { function loadRedirects() { const source = fs.readFileSync(REDIRECTS_FILE, 'utf-8'); const constants = fs.readFileSync(CONSTANTS_FILE, 'utf-8'); - const rssUrl = constants.match(/TECHNICAL_CHANGELOG_RSS_URL\s*=\s*['"]([^'"]+)['"]/)?.[1] ?? ''; + const rssUrl = + constants.match( + /TECHNICAL_CHANGELOG_RSS_URL\s*=\s*['"]([^'"]+)['"]/ + )?.[1] ?? ''; const script = source .replace(/^import .*$/gm, '') .replace(/^export const /gm, 'const ') .replace(/module\.exports\s*=\s*\{[\s\S]*?\};?/g, ''); - const sandbox = { TECHNICAL_CHANGELOG_RSS_URL: rssUrl }; + const sandbox = {TECHNICAL_CHANGELOG_RSS_URL: rssUrl}; vm.runInNewContext(`${script}\nresult = updatedRedirectsData;`, sandbox); // Line number of each entry, for the report. Entries are written one @@ -91,7 +96,10 @@ function buildHeadingsByRoute() { for (const file of listFiles(DOCS_DIR, ['.mdx'])) { const route = routeFor(file); if (headingsByRoute.has(route)) continue; - headingsByRoute.set(route, extractHeadings(fs.readFileSync(path.join(DOCS_DIR, file), 'utf-8'))); + headingsByRoute.set( + route, + extractHeadings(fs.readFileSync(path.join(DOCS_DIR, file), 'utf-8')) + ); } return headingsByRoute; } @@ -100,7 +108,7 @@ function buildHeadingsByRoute() { function splitUrl(url) { const [pathAndQuery, fragment = ''] = url.split('#'); const pathname = pathAndQuery.split('?')[0].replace(/\/+$/, '') || '/'; - return { pathname, fragment: decodeURIComponent(fragment) }; + return {pathname, fragment: decodeURIComponent(fragment)}; } function isPublicFile(pathname) { @@ -119,16 +127,26 @@ function findBrokenRedirects(redirects, headingsByRoute) { // dead code and cannot break anything. const firstBySource = new Map(); for (const redirect of redirects) { - if (!firstBySource.has(redirect.source)) firstBySource.set(redirect.source, redirect); + if (!firstBySource.has(redirect.source)) + firstBySource.set(redirect.source, redirect); } - const report = (redirect, problem) => findings.push({ ...redirect, problem }); + const report = (redirect, problem) => findings.push({...redirect, problem}); for (const redirect of redirects) { if (firstBySource.get(redirect.source) !== redirect) continue; const source = splitUrl(redirect.source); - if (!source.fragment && headingsByRoute.has(source.pathname)) { - report(redirect, 'source is an existing page; visitors to it are redirected away'); + if (source.fragment) { + // Browsers strip #fragments before sending a request, so no server + // can ever match this source. Nothing else about the entry matters. + report( + redirect, + 'Source has a #fragment, which browsers never send, so this redirect can never match' + ); + continue; + } + if (headingsByRoute.has(source.pathname)) { + report(redirect, 'Source overshadows a page that exists'); } if (isExternal(redirect.destination)) continue; @@ -137,9 +155,15 @@ function findBrokenRedirects(redirects, headingsByRoute) { let destination = splitUrl(redirect.destination); let hops = 0; const visited = new Set([redirect.source]); - while (firstBySource.has(destination.pathname) && !headingsByRoute.has(destination.pathname)) { + while ( + firstBySource.has(destination.pathname) && + !headingsByRoute.has(destination.pathname) + ) { if (visited.has(destination.pathname) || ++hops > MAX_CHAIN_HOPS) { - report(redirect, `redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops`); + report( + redirect, + `Redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops` + ); destination = undefined; break; } @@ -151,19 +175,28 @@ function findBrokenRedirects(redirects, headingsByRoute) { } const nextUrl = splitUrl(next); // A hop without its own fragment keeps the fragment we have - destination = { pathname: nextUrl.pathname, fragment: nextUrl.fragment || destination.fragment }; + destination = { + pathname: nextUrl.pathname, + fragment: nextUrl.fragment || destination.fragment + }; } if (!destination) continue; const headings = headingsByRoute.get(destination.pathname); if (!headings) { if (!isPublicFile(destination.pathname)) { - report(redirect, `destination page ${destination.pathname} does not exist`); + report( + redirect, + `Destination page ${destination.pathname} does not exist` + ); } continue; } if (destination.fragment && !headings.has(destination.fragment)) { - report(redirect, `heading #${destination.fragment} not found on ${destination.pathname}`); + report( + redirect, + `Heading #${destination.fragment} not found on page ${destination.pathname}` + ); } } return findings; @@ -175,7 +208,9 @@ function findingKey(finding) { } function withoutBaseline(findings, baselineFile) { - const baseline = new Set(JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey)); + const baseline = new Set( + JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey) + ); return findings.filter(finding => !baseline.has(findingKey(finding))); } @@ -185,7 +220,7 @@ function formatText(findings) { return `βœ… No redirects ${scope}\n`; } const lines = [`❌ ${findings.length} redirect(s) ${scope}:`, '']; - for (const { source, destination, line, problem } of findings) { + for (const {source, destination, line, problem} of findings) { lines.push( ` ${REDIRECTS_PATH}${line ? `:${line}` : ''}`, ` ${source} -> ${destination}`, @@ -200,6 +235,27 @@ function linkTo(text, url) { return url ? `[${text}](${url})` : text; } +// Report sections, most urgent first: a shadowed page is unreachable today +const PROBLEM_ORDER = [ + 'Source overshadows', + 'Source has a #fragment', + 'Destination page', + 'Heading', + 'Redirect loop' +]; + +// Map of problem -> findings with that problem, ordered by PROBLEM_ORDER +function groupByProblem(findings) { + const rank = problem => + PROBLEM_ORDER.findIndex(prefix => problem.startsWith(prefix)); + const groups = new Map(); + for (const finding of findings) { + if (!groups.has(finding.problem)) groups.set(finding.problem, []); + groups.get(finding.problem).push(finding); + } + return new Map([...groups].sort(([a], [b]) => rank(a) - rank(b))); +} + // Body for a pull request comment function formatMarkdown(findings) { if (findings.length === 0) { @@ -211,33 +267,45 @@ function formatMarkdown(findings) { const lines = [ `### ❌ This PR breaks ${findings.length} redirect(s)`, '', - 'Redirects exist so external traffic (search results, bookmarks) to an old URL ' + - 'still reaches a page. Each one must point at a page and heading that exist, ' + - 'and must not shadow a page that exists.', + 'Redirects are used so external traffic (links inside old versions of our product, ' + + 'bookmarks, search results, etc.) to old URLs still reaches a relevant page.', + '', + 'Each redirect entry must have:', '', - '- If this PR moved or renamed the redirect destination, then update the ' + - 'redirect with the updated destination', + '- A destination which points to a page that currently exists', '', - '- If this PR removed the destination, then either update the destination to ' + - 'the next most relevant page, or remove it to leave the user with our 404 page', + '- A source which does not overshadow a page that exists', '', - 'Redirects are not to be used for internal links (tech debt snowball), internal ' + - 'links must be fixed; the "Check links" comment lists any this PR broke.', + 'If this PR moved or renamed the redirect destination, then update the redirect ' + + 'with the updated destination', + '', + 'If this PR removed the destination, then either update the destination to the ' + + 'next most relevant page, or remove it to leave the user with our 404 page', + '', + 'Do not use redirects for broken internal links, internal links must be fixed ' + + 'properly to tame the tech debt snowball no one wants to deal with; the ' + + '"Check links" PR check comment lists the links this PR broke, if any.', '', linkTo(`**\`${REDIRECTS_PATH}\`**`, fileUrl) ]; - // Each entry is shown as it appears in the redirects file, so it is easy to find there - for (const { source, destination, line, problem } of findings) { - const where = line ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) : 'entry'; - lines.push( - `- ${where}: ${problem}`, - ' ```ts', - ` source: '${source}',`, - ` destination: '${destination}'`, - ' ```' - ); + // One section per problem, in PROBLEM_ORDER. Each entry is shown as it + // appears in the redirects file, so it is easy to find there. + for (const [problem, entries] of groupByProblem(findings)) { + lines.push('', `#### ${problem}`); + for (const {source, destination, line} of entries) { + lines.push( + `- ${line ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) : 'entry'}`, + ' ```ts', + ` source: '${source}',`, + ` destination: '${destination}'`, + ' ```' + ); + } } - lines.push('', 'Reproduce locally with `pnpm check-redirects` (see `dev/check-redirects.mjs`).'); + lines.push( + '', + 'Reproduce locally with `pnpm check-redirects` (see `dev/check-redirects.mjs`)' + ); return lines.join('\n') + '\n'; } @@ -250,7 +318,9 @@ const FORMATTERS = { function main() { const format = FORMATTERS[FORMAT]; if (!format) { - throw new Error(`Unknown --format "${FORMAT}"; use text, json, or markdown`); + throw new Error( + `Unknown --format "${FORMAT}"; use text, json, or markdown` + ); } let findings = findBrokenRedirects(loadRedirects(), buildHeadingsByRoute()); From de75e95c683315f57986b186a6802c4bdc80234b Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:24:04 -0600 Subject: [PATCH 23/29] check-redirects: fixed section headings per problem, entries in line order Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 57 +++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 7d57cdb76..4ef04df0a 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -48,6 +48,17 @@ const DOCS_DIR = path.join(ROOT_DIR, 'docs'); const PUBLIC_DIR = path.join(ROOT_DIR, 'public'); const MAX_CHAIN_HOPS = 10; +// Report sections, most urgent first: a shadowed page is unreachable today +const PROBLEM = { + shadowsPage: 'Source overshadows a docs page that already exists', + fragmentSource: + 'Source has a #fragment, which browsers never send, so this redirect can never match', + missingPage: 'Destination page does not exist', + missingHeading: 'Destination heading does not exist', + loop: `Redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops` +}; +const PROBLEM_ORDER = Object.values(PROBLEM); + function flagValue(name) { const index = args.indexOf(name); return index === -1 ? undefined : args[index + 1]; @@ -139,14 +150,11 @@ function findBrokenRedirects(redirects, headingsByRoute) { if (source.fragment) { // Browsers strip #fragments before sending a request, so no server // can ever match this source. Nothing else about the entry matters. - report( - redirect, - 'Source has a #fragment, which browsers never send, so this redirect can never match' - ); + report(redirect, PROBLEM.fragmentSource); continue; } if (headingsByRoute.has(source.pathname)) { - report(redirect, 'Source overshadows a page that exists'); + report(redirect, PROBLEM.shadowsPage); } if (isExternal(redirect.destination)) continue; @@ -160,10 +168,7 @@ function findBrokenRedirects(redirects, headingsByRoute) { !headingsByRoute.has(destination.pathname) ) { if (visited.has(destination.pathname) || ++hops > MAX_CHAIN_HOPS) { - report( - redirect, - `Redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops` - ); + report(redirect, PROBLEM.loop); destination = undefined; break; } @@ -185,18 +190,12 @@ function findBrokenRedirects(redirects, headingsByRoute) { const headings = headingsByRoute.get(destination.pathname); if (!headings) { if (!isPublicFile(destination.pathname)) { - report( - redirect, - `Destination page ${destination.pathname} does not exist` - ); + report(redirect, PROBLEM.missingPage); } continue; } if (destination.fragment && !headings.has(destination.fragment)) { - report( - redirect, - `Heading #${destination.fragment} not found on page ${destination.pathname}` - ); + report(redirect, PROBLEM.missingHeading); } } return findings; @@ -235,25 +234,15 @@ function linkTo(text, url) { return url ? `[${text}](${url})` : text; } -// Report sections, most urgent first: a shadowed page is unreachable today -const PROBLEM_ORDER = [ - 'Source overshadows', - 'Source has a #fragment', - 'Destination page', - 'Heading', - 'Redirect loop' -]; - -// Map of problem -> findings with that problem, ordered by PROBLEM_ORDER +// Map of problem -> its findings in line order, sections in PROBLEM_ORDER function groupByProblem(findings) { - const rank = problem => - PROBLEM_ORDER.findIndex(prefix => problem.startsWith(prefix)); - const groups = new Map(); - for (const finding of findings) { - if (!groups.has(finding.problem)) groups.set(finding.problem, []); - groups.get(finding.problem).push(finding); + const groups = new Map(PROBLEM_ORDER.map(problem => [problem, []])); + for (const finding of findings) groups.get(finding.problem).push(finding); + for (const [problem, entries] of groups) { + if (entries.length === 0) groups.delete(problem); + else entries.sort((a, b) => (a.line ?? 0) - (b.line ?? 0)); } - return new Map([...groups].sort(([a], [b]) => rank(a) - rank(b))); + return groups; } // Body for a pull request comment From 49aca43a7d30988959bb3fcf2235b75cba711add Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:30:13 -0600 Subject: [PATCH 24/29] check-redirects: report chained and duplicate-source entries, teach the fix under each section Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 157 +++++++++++++++++++++++----------------- 1 file changed, 90 insertions(+), 67 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 4ef04df0a..506cbe8f9 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -5,13 +5,14 @@ * * Redirects exist so external traffic to an old URL still reaches a page, so * each one must be correct. Checks, for every entry: - * - the destination page exists under docs/ (or is a file under public/), - * following chains through other redirects - * - when the destination has a #fragment, the heading exists on that page * - the source does not shadow an existing page (the middleware would redirect * visitors away from a page that exists) * - the source has no #fragment: browsers never send fragments, so such an * entry can never match + * - the source has no earlier entry: the middleware uses the first match only + * - the destination is a page, not another redirect + * - the destination page exists under docs/ (or is a file under public/) + * - when the destination has a #fragment, the heading exists on that page * * External (http) destinations are not checked. * @@ -46,18 +47,39 @@ const REDIRECTS_FILE = path.join(ROOT_DIR, REDIRECTS_PATH); const CONSTANTS_FILE = path.join(ROOT_DIR, 'src/data/constants.ts'); const DOCS_DIR = path.join(ROOT_DIR, 'docs'); const PUBLIC_DIR = path.join(ROOT_DIR, 'public'); -const MAX_CHAIN_HOPS = 10; - -// Report sections, most urgent first: a shadowed page is unreachable today +// Report sections, most urgent first: a shadowed page is unreachable today. +// `fix` teaches the author what a correct entry looks like, where it applies. const PROBLEM = { - shadowsPage: 'Source overshadows a docs page that already exists', - fragmentSource: - 'Source has a #fragment, which browsers never send, so this redirect can never match', - missingPage: 'Destination page does not exist', - missingHeading: 'Destination heading does not exist', - loop: `Redirect loop or chain longer than ${MAX_CHAIN_HOPS} hops` + shadowsPage: { + heading: 'Source overshadows a docs page that already exists', + fix: 'Visitors to that page are redirected away from it. Remove the redirect, or rename the page.' + }, + fragmentSource: { + heading: 'Source has a #fragment, so this redirect can never match', + fix: + 'Browsers never send the #fragment to the server. Use the page path alone as the source; ' + + "when the destination has no #fragment, the browser keeps the visitor's own." + }, + duplicateSource: { + heading: + 'Source already has an earlier entry, so this one is never used', + fix: 'Only the first entry for a source matches. Update that entry instead of adding another.' + }, + chained: { + heading: 'Destination is another redirect', + fix: 'Point straight at the final page (named after each line); every hop costs the visitor a round trip.' + }, + missingPage: { + heading: 'Destination page does not exist', + fix: + 'Point at the page that replaced it, or remove the entry if there is no replacement ' + + '(visitors then get the 404 page).' + }, + missingHeading: { + heading: 'Destination heading does not exist', + fix: "Use the heading's current slug, or drop the #fragment to land at the top of the page." + } }; -const PROBLEM_ORDER = Object.values(PROBLEM); function flagValue(name) { const index = args.indexOf(name); @@ -131,62 +153,43 @@ function isExternal(url) { return /^https?:\/\//.test(url); } -// Every incorrect redirect: [{ source, destination, line, problem }] +// Every incorrect redirect: [{ source, destination, line, problem, detail? }] function findBrokenRedirects(redirects, headingsByRoute) { const findings = []; - // Only the first entry for a source is reachable; later duplicates are - // dead code and cannot break anything. const firstBySource = new Map(); for (const redirect of redirects) { if (!firstBySource.has(redirect.source)) firstBySource.set(redirect.source, redirect); } - const report = (redirect, problem) => findings.push({...redirect, problem}); + const report = (redirect, problem, detail) => + findings.push({...redirect, problem: problem.heading, detail}); + const isRedirect = pathname => + firstBySource.has(pathname) && !headingsByRoute.has(pathname); for (const redirect of redirects) { - if (firstBySource.get(redirect.source) !== redirect) continue; - + if (firstBySource.get(redirect.source) !== redirect) { + report(redirect, PROBLEM.duplicateSource); + continue; + } const source = splitUrl(redirect.source); if (source.fragment) { - // Browsers strip #fragments before sending a request, so no server - // can ever match this source. Nothing else about the entry matters. report(redirect, PROBLEM.fragmentSource); continue; } if (headingsByRoute.has(source.pathname)) { report(redirect, PROBLEM.shadowsPage); } - if (isExternal(redirect.destination)) continue; - // Follow redirect chains to the page a visitor finally lands on - let destination = splitUrl(redirect.destination); - let hops = 0; - const visited = new Set([redirect.source]); - while ( - firstBySource.has(destination.pathname) && - !headingsByRoute.has(destination.pathname) - ) { - if (visited.has(destination.pathname) || ++hops > MAX_CHAIN_HOPS) { - report(redirect, PROBLEM.loop); - destination = undefined; - break; - } - visited.add(destination.pathname); - const next = firstBySource.get(destination.pathname).destination; - if (isExternal(next)) { - destination = undefined; - break; - } - const nextUrl = splitUrl(next); - // A hop without its own fragment keeps the fragment we have - destination = { - pathname: nextUrl.pathname, - fragment: nextUrl.fragment || destination.fragment - }; + const destination = splitUrl(redirect.destination); + if (isRedirect(destination.pathname)) { + report( + redirect, + PROBLEM.chained, + finalDestination(destination.pathname) + ); + continue; } - if (!destination) continue; - const headings = headingsByRoute.get(destination.pathname); if (!headings) { if (!isPublicFile(destination.pathname)) { @@ -198,6 +201,18 @@ function findBrokenRedirects(redirects, headingsByRoute) { report(redirect, PROBLEM.missingHeading); } } + + // Where a visitor to `pathname` finally lands, e.g. "ends at /new-page" + function finalDestination(pathname) { + const visited = new Set(); + while (isRedirect(pathname) && !visited.has(pathname)) { + visited.add(pathname); + pathname = firstBySource.get(pathname).destination; + if (isExternal(pathname)) return `ends at ${pathname}`; + pathname = splitUrl(pathname).pathname; + } + return visited.has(pathname) ? 'redirect loop' : `ends at ${pathname}`; + } return findings; } @@ -219,11 +234,11 @@ function formatText(findings) { return `βœ… No redirects ${scope}\n`; } const lines = [`❌ ${findings.length} redirect(s) ${scope}:`, '']; - for (const {source, destination, line, problem} of findings) { + for (const {source, destination, line, problem, detail} of findings) { lines.push( ` ${REDIRECTS_PATH}${line ? `:${line}` : ''}`, ` ${source} -> ${destination}`, - ` ${problem}`, + ` ${problem}${detail ? ` (${detail})` : ''}`, '' ); } @@ -234,9 +249,11 @@ function linkTo(text, url) { return url ? `[${text}](${url})` : text; } -// Map of problem -> its findings in line order, sections in PROBLEM_ORDER +// Map of problem heading -> its findings in line order, sections in PROBLEM order function groupByProblem(findings) { - const groups = new Map(PROBLEM_ORDER.map(problem => [problem, []])); + const groups = new Map( + Object.values(PROBLEM).map(({heading}) => [heading, []]) + ); for (const finding of findings) groups.get(finding.problem).push(finding); for (const [problem, entries] of groups) { if (entries.length === 0) groups.delete(problem); @@ -259,17 +276,17 @@ function formatMarkdown(findings) { 'Redirects are used so external traffic (links inside old versions of our product, ' + 'bookmarks, search results, etc.) to old URLs still reaches a relevant page.', '', - 'Each redirect entry must have:', - '', - '- A destination which points to a page that currently exists', + 'A correct entry maps the old page path, exactly as the browser requests it, ' + + 'straight to a page that exists today, with an optional #heading that exists on that page:', '', - '- A source which does not overshadow a page that exists', + '```ts', + '{', + "\tsource: '/old/section/page',", + "\tdestination: '/new/section/page#heading-slug'", + '},', + '```', '', - 'If this PR moved or renamed the redirect destination, then update the redirect ' + - 'with the updated destination', - '', - 'If this PR removed the destination, then either update the destination to the ' + - 'next most relevant page, or remove it to leave the user with our 404 page', + 'Each section below says how to fix the entries listed under it.', '', 'Do not use redirects for broken internal links, internal links must be fixed ' + 'properly to tame the tech debt snowball no one wants to deal with; the ' + @@ -277,13 +294,19 @@ function formatMarkdown(findings) { '', linkTo(`**\`${REDIRECTS_PATH}\`**`, fileUrl) ]; - // One section per problem, in PROBLEM_ORDER. Each entry is shown as it - // appears in the redirects file, so it is easy to find there. + // One section per problem with its fix. Each entry is shown as it appears + // in the redirects file, so it is easy to find there. + const fixes = new Map( + Object.values(PROBLEM).map(({heading, fix}) => [heading, fix]) + ); for (const [problem, entries] of groupByProblem(findings)) { - lines.push('', `#### ${problem}`); - for (const {source, destination, line} of entries) { + lines.push('', `#### ${problem}`, '', fixes.get(problem), ''); + for (const {source, destination, line, detail} of entries) { + const where = line + ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) + : 'entry'; lines.push( - `- ${line ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) : 'entry'}`, + `- ${where}${detail ? `: ${detail}` : ''}`, ' ```ts', ` source: '${source}',`, ` destination: '${destination}'`, From 340bd454c91b5e14182e88eb1be2842188d58e83 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:07:21 -0600 Subject: [PATCH 25/29] check-redirects: flag /docs-prefixed paths, show final destination inside the entry block, reword fixes Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 51 ++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 506cbe8f9..d867a0459 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -10,6 +10,8 @@ * - the source has no #fragment: browsers never send fragments, so such an * entry can never match * - the source has no earlier entry: the middleware uses the first match only + * - neither path starts with /docs: the middleware strips that prefix from + * requests and adds it to destinations * - the destination is a page, not another redirect * - the destination page exists under docs/ (or is a file under public/) * - when the destination has a #fragment, the heading exists on that page @@ -51,14 +53,25 @@ const PUBLIC_DIR = path.join(ROOT_DIR, 'public'); // `fix` teaches the author what a correct entry looks like, where it applies. const PROBLEM = { shadowsPage: { - heading: 'Source overshadows a docs page that already exists', - fix: 'Visitors to that page are redirected away from it. Remove the redirect, or rename the page.' + heading: 'Source overshadows a docs page that exists', + fix: + "Redirects take precedence over pages, so visitors to that page's URL are redirected away from it. " + + 'Update or remove the redirect or the page to remove the conflict.' }, fragmentSource: { heading: 'Source has a #fragment, so this redirect can never match', fix: - 'Browsers never send the #fragment to the server. Use the page path alone as the source; ' + - "when the destination has no #fragment, the browser keeps the visitor's own." + 'Use the page path alone as the source. #fragments are processed in the browser, so browsers ' + + 'never send them to web servers. If the redirect destination has a #fragment, it takes precedence, ' + + "otherwise if the customer clicked a link which has a #fragment, it'll be kept and tried on the " + + 'destination page.' + }, + docsPrefix: { + heading: 'Source or destination starts with /docs', + fix: + 'Write paths without the /docs prefix. The site removes /docs from the requested URL before ' + + 'matching sources, and adds it back in front of the destination, so a /docs/... source never ' + + 'matches and a /docs/... destination lands on /docs/docs/....' }, duplicateSource: { heading: @@ -67,17 +80,21 @@ const PROBLEM = { }, chained: { heading: 'Destination is another redirect', - fix: 'Point straight at the final page (named after each line); every hop costs the visitor a round trip.' + fix: + "Chained redirects cost the customer's browser a round trip, slow down their page load time, " + + 'and frustrate them. They also make the redirects file impossible to maintain, and make it too ' + + "easy to create redirect loops. Change the rule's destination to the final destination." }, missingPage: { heading: 'Destination page does not exist', fix: - 'Point at the page that replaced it, or remove the entry if there is no replacement ' + - '(visitors then get the 404 page).' + 'Point at the page that replaced it, or remove the rule if there is no replacement page; ' + + "visitors then get our fancy 404 page, with links they can click to find where they're trying " + + 'to go, and the search bar.' }, missingHeading: { heading: 'Destination heading does not exist', - fix: "Use the heading's current slug, or drop the #fragment to land at the top of the page." + fix: "Use the heading's correct anchor, or drop the #fragment to land the customer at the top of the page." } }; @@ -179,6 +196,13 @@ function findBrokenRedirects(redirects, headingsByRoute) { if (headingsByRoute.has(source.pathname)) { report(redirect, PROBLEM.shadowsPage); } + if ( + source.pathname.startsWith('/docs/') || + redirect.destination.startsWith('/docs/') + ) { + report(redirect, PROBLEM.docsPrefix); + continue; + } if (isExternal(redirect.destination)) continue; const destination = splitUrl(redirect.destination); @@ -202,16 +226,16 @@ function findBrokenRedirects(redirects, headingsByRoute) { } } - // Where a visitor to `pathname` finally lands, e.g. "ends at /new-page" + // Where a visitor to `pathname` finally lands function finalDestination(pathname) { const visited = new Set(); while (isRedirect(pathname) && !visited.has(pathname)) { visited.add(pathname); pathname = firstBySource.get(pathname).destination; - if (isExternal(pathname)) return `ends at ${pathname}`; + if (isExternal(pathname)) return pathname; pathname = splitUrl(pathname).pathname; } - return visited.has(pathname) ? 'redirect loop' : `ends at ${pathname}`; + return visited.has(pathname) ? 'none, redirect loop' : pathname; } return findings; } @@ -238,7 +262,7 @@ function formatText(findings) { lines.push( ` ${REDIRECTS_PATH}${line ? `:${line}` : ''}`, ` ${source} -> ${destination}`, - ` ${problem}${detail ? ` (${detail})` : ''}`, + ` ${problem}${detail ? ` (final destination: ${detail})` : ''}`, '' ); } @@ -306,10 +330,11 @@ function formatMarkdown(findings) { ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) : 'entry'; lines.push( - `- ${where}${detail ? `: ${detail}` : ''}`, + `- ${where}`, ' ```ts', ` source: '${source}',`, ` destination: '${destination}'`, + ...(detail ? [` final destination: ${detail}`] : []), ' ```' ); } From cbcca355b79d7be717cdc3d17b1dbfc05d750e32 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:19:46 -0600 Subject: [PATCH 26/29] check-redirects: reword intro Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index d867a0459..520a5388e 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -297,8 +297,8 @@ function formatMarkdown(findings) { const lines = [ `### ❌ This PR breaks ${findings.length} redirect(s)`, '', - 'Redirects are used so external traffic (links inside old versions of our product, ' + - 'bookmarks, search results, etc.) to old URLs still reaches a relevant page.', + 'Redirects are used so inbound traffic from external sources (links inside old versions of ' + + 'our product, bookmarks, search results, etc.) to old doc pages still reaches a relevant page.', '', 'A correct entry maps the old page path, exactly as the browser requests it, ' + 'straight to a page that exists today, with an optional #heading that exists on that page:', From b315c8891ed8b74f80342b6b53416bf7e60e0380 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:20:31 -0600 Subject: [PATCH 27/29] check-redirects: reword example intro Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 520a5388e..fac19713c 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -301,7 +301,7 @@ function formatMarkdown(findings) { 'our product, bookmarks, search results, etc.) to old doc pages still reaches a relevant page.', '', 'A correct entry maps the old page path, exactly as the browser requests it, ' + - 'straight to a page that exists today, with an optional #heading that exists on that page:', + 'straight to a page that exists today, with an optional #heading that exists on the destination page:', '', '```ts', '{', From 039bbfabd5187a9d346bc8345d2e71b98a67201a Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:28:03 -0600 Subject: [PATCH 28/29] check-redirects: reword fixes Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index fac19713c..984bd7cf0 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -62,9 +62,9 @@ const PROBLEM = { heading: 'Source has a #fragment, so this redirect can never match', fix: 'Use the page path alone as the source. #fragments are processed in the browser, so browsers ' + - 'never send them to web servers. If the redirect destination has a #fragment, it takes precedence, ' + - "otherwise if the customer clicked a link which has a #fragment, it'll be kept and tried on the " + - 'destination page.' + 'never send them to web servers.\n\n' + + 'If the redirect destination has a #fragment, it takes precedence, otherwise if the customer ' + + "clicked a link which has a #fragment, it'll be kept and tried on the destination page." }, docsPrefix: { heading: 'Source or destination starts with /docs', @@ -88,7 +88,7 @@ const PROBLEM = { missingPage: { heading: 'Destination page does not exist', fix: - 'Point at the page that replaced it, or remove the rule if there is no replacement page; ' + + 'Set the redirect destination to the page that replaced it, or remove the rule if there is no replacement page; ' + "visitors then get our fancy 404 page, with links they can click to find where they're trying " + 'to go, and the search bar.' }, @@ -310,7 +310,7 @@ function formatMarkdown(findings) { '},', '```', '', - 'Each section below says how to fix the entries listed under it.', + 'Each section below explains how to fix the entries listed under it.', '', 'Do not use redirects for broken internal links, internal links must be fixed ' + 'properly to tame the tech debt snowball no one wants to deal with; the ' + From 4845f0c5caa420aa2e0290a9a894f75cbe07c672 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 10 Sep 2026 00:35:46 -0600 Subject: [PATCH 29/29] check-redirects: classify fragment sources before duplicates Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3 Co-authored-by: Amp --- dev/check-redirects.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 984bd7cf0..c6f2755ea 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -184,15 +184,15 @@ function findBrokenRedirects(redirects, headingsByRoute) { firstBySource.has(pathname) && !headingsByRoute.has(pathname); for (const redirect of redirects) { - if (firstBySource.get(redirect.source) !== redirect) { - report(redirect, PROBLEM.duplicateSource); - continue; - } const source = splitUrl(redirect.source); if (source.fragment) { report(redirect, PROBLEM.fragmentSource); continue; } + if (firstBySource.get(redirect.source) !== redirect) { + report(redirect, PROBLEM.duplicateSource); + continue; + } if (headingsByRoute.has(source.pathname)) { report(redirect, PROBLEM.shadowsPage); }