diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml new file mode 100644 index 000000000..f2c5b5132 --- /dev/null +++ b/.github/workflows/check-links.yml @@ -0,0 +1,99 @@ +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: + +# 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-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 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: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + 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 + 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 + 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" \ + --changed-files "$RUNNER_TEMP/changed-files.txt" \ + --link-base "$LINK_BASE" > "$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 links 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 links + if: steps.check.outputs.broken == 'true' + run: exit 1 diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml new file mode 100644 index 000000000..c3f0e24b2 --- /dev/null +++ b/.github/workflows/check-redirects.yml @@ -0,0 +1,97 @@ +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: + +# 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 + 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 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: + 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 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" \ + --link-base "$LINK_BASE" > "$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 redirects an earlier revision of this PR broke 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 breaks redirects + if: steps.check.outputs.broken == 'true' + run: exit 1 diff --git a/AGENTS.md b/AGENTS.md index 9a5e9b001..1af2ac832 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,6 +6,8 @@ - **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`). 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 2a8b24093..18f36c42b 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -8,106 +8,183 @@ * * 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) + * --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. */ 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); -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 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 +// `filePathPattern` in contentlayer.config.ts. +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. +export 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; const SRC_ATTR_REGEX = /src=["']([^"']+)["']/g; -// Extract headings from MDX content to build anchor map -function extractHeadings(content) { +// 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 +export 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 - const contentWithoutCode = content.replace(/```[\s\S]*?```/g, ''); + const contentWithoutCode = stripFencedCodeBlocks(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())); } + while ((match = explicitAnchorRegex.exec(contentWithoutCode)) !== null) { + headings.add(match[1]); + } + return headings; } +// Site route for a file under docs/: foo/bar.mdx -> /foo/bar, foo/index.mdx -> /foo, index.mdx -> / +export 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('**/*.mdx', { cwd: DOCS_DIR }); +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(); 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); - // Route path (without .mdx extension) - const routePath = '/' + file.replace(/\.mdx$/, '').replace(/\/index$/, ''); + const routePath = routeFor(file); + if (pathMap.has(routePath)) continue; // Also allow trailing slash variant pathMap.set(routePath, fullPath); pathMap.set(routePath + '/', fullPath); - - // 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); + routesByLowerCase.set(routePath.toLowerCase(), routePath); headingsMap.set(routePath, headings); headingsMap.set(routePath + '/', headings); } - return { pathMap, headingsMap }; + return { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase: buildAssetMap() }; } -// Check if a path exists in public directory -function checkPublicPath(linkPath) { - const publicPath = path.join(path.dirname(__dirname), '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. +function buildAssetMap() { + const assetsByLowerCase = new Map(); + for (const dir of ['public', 'docs']) { + for (const file of listFiles(path.join(ROOT_DIR, dir))) { + const linkPath = '/' + file; + assetsByLowerCase.set(linkPath.toLowerCase(), linkPath); + } + } + return assetsByLowerCase; } // Parse and validate links in a single file 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; @@ -137,7 +214,7 @@ function extractLinks(content, filePath) { } // Check if a link is valid -function validateLink(link, currentFile, pathMap, headingsMap) { +function validateLink(link, currentFile, { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase }) { const { url } = link; // Skip external links, mailto, tel, javascript, etc. @@ -164,10 +241,7 @@ function validateLink(link, currentFile, pathMap, headingsMap) { 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 = headingsByFile.get(currentFile); if (headings && !headings.has(anchor)) { return `Anchor "${anchor}" not found in current file`; @@ -211,80 +285,187 @@ function validateLink(link, currentFile, pathMap, headingsMap) { return null; } - // Check if it's a public asset - if (checkPublicPath(resolvedPath)) { + // 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 (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}"`; } return `Page not found: "${resolvedPath}"`; } -async function main() { - console.log('πŸ” Checking for dead links in MDX files...\n'); +// Find every broken link: [{ file, line, url, error }] +function findBrokenLinks() { + const maps = buildPathMap(); + const findings = []; - const { pathMap, headingsMap } = await buildPathMap(); - const files = await glob('**/*.mdx', { cwd: DOCS_DIR }); - - let totalErrors = 0; - const errors = []; - - for (const file of files) { + for (const file of listFiles(DOCS_DIR, SOURCE_EXTENSIONS)) { 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'; +} + +function linkTo(text, url) { + return url ? `[${text}](${url})` : text; +} + +// 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 + const fileUrl = LINK_BASE && `${LINK_BASE}/${file}?plain=1`; + lines.push(linkTo(`**\`${file}\`**`, fileUrl)); + for (const { line, url, error } of fileFindings) { + lines.push(`- ${linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`)}: \`${url}\` β€” ${error}`); + } + 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'; + } - 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)`, '']; + 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`).', + '', + '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'; +} + +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`); } - console.log('\n'); - process.exit(1); + if (FORMAT === 'text') { + console.log('πŸ” Checking for dead links in MDX files...\n'); + } + + let findings = findBrokenLinks(); + if (BASELINE_FILE) { + findings = withoutBaseline(findings, BASELINE_FILE); + } + + process.stdout.write(format(findings)); + 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 and dev/check-redirects.mjs +// import the exported helpers. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main(); +} diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs new file mode 100644 index 000000000..c6f2755ea --- /dev/null +++ b/dev/check-redirects.mjs @@ -0,0 +1,372 @@ +#!/usr/bin/env node + +/** + * 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: + * - 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 + * - 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 + * + * External (http) destinations are not checked. + * + * 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/ + * + * Exits 1 when any finding is reported. + */ + +import fs from 'fs'; +import path from 'path'; +import vm from 'vm'; +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); +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_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'); +// 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: { + 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: + 'Use the page path alone as the source. #fragments are processed in the browser, so browsers ' + + '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', + 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: + '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: + "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: + '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.' + }, + missingHeading: { + heading: 'Destination heading does not exist', + fix: "Use the heading's correct anchor, or drop the #fragment to land the customer at the top of the page." + } +}; + +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 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}; + 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, index) => { + if (/^\s*source:/.test(line)) sourceLines.push(index + 1); + }); + const haveLines = sourceLines.length === sandbox.result.length; + + return sandbox.result.map((redirect, index) => ({ + source: redirect.source, + destination: redirect.destination, + line: haveLines ? sourceLines[index] : undefined + })); +} + +// 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 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; +} + +// '/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(pathname) { + const fullPath = path.join(PUBLIC_DIR, pathname); + return fullPath.startsWith(PUBLIC_DIR) && fs.existsSync(fullPath); +} + +function isExternal(url) { + return /^https?:\/\//.test(url); +} + +// Every incorrect redirect: [{ source, destination, line, problem, detail? }] +function findBrokenRedirects(redirects, headingsByRoute) { + const findings = []; + const firstBySource = new Map(); + for (const redirect of redirects) { + if (!firstBySource.has(redirect.source)) + firstBySource.set(redirect.source, redirect); + } + 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) { + 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); + } + if ( + source.pathname.startsWith('/docs/') || + redirect.destination.startsWith('/docs/') + ) { + report(redirect, PROBLEM.docsPrefix); + continue; + } + if (isExternal(redirect.destination)) continue; + + const destination = splitUrl(redirect.destination); + if (isRedirect(destination.pathname)) { + report( + redirect, + PROBLEM.chained, + finalDestination(destination.pathname) + ); + continue; + } + const headings = headingsByRoute.get(destination.pathname); + if (!headings) { + if (!isPublicFile(destination.pathname)) { + report(redirect, PROBLEM.missingPage); + } + continue; + } + if (destination.fragment && !headings.has(destination.fragment)) { + report(redirect, PROBLEM.missingHeading); + } + } + + // 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 pathname; + pathname = splitUrl(pathname).pathname; + } + return visited.has(pathname) ? 'none, redirect loop' : pathname; + } + return findings; +} + +// 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 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 formatText(findings) { + const scope = BASELINE_FILE ? 'broken by this change' : 'broken'; + if (findings.length === 0) { + return `βœ… No redirects ${scope}\n`; + } + const lines = [`❌ ${findings.length} redirect(s) ${scope}:`, '']; + for (const {source, destination, line, problem, detail} of findings) { + lines.push( + ` ${REDIRECTS_PATH}${line ? `:${line}` : ''}`, + ` ${source} -> ${destination}`, + ` ${problem}${detail ? ` (final destination: ${detail})` : ''}`, + '' + ); + } + return lines.join('\n'); +} + +function linkTo(text, url) { + return url ? `[${text}](${url})` : text; +} + +// Map of problem heading -> its findings in line order, sections in PROBLEM order +function groupByProblem(findings) { + 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); + else entries.sort((a, b) => (a.line ?? 0) - (b.line ?? 0)); + } + return groups; +} + +// Body for a pull request comment +function formatMarkdown(findings) { + if (findings.length === 0) { + return '### βœ… This PR breaks no redirects\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 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 the destination page:', + '', + '```ts', + '{', + "\tsource: '/old/section/page',", + "\tdestination: '/new/section/page#heading-slug'", + '},', + '```', + '', + '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 ' + + '"Check links" PR check comment lists the links this PR broke, if any.', + '', + linkTo(`**\`${REDIRECTS_PATH}\`**`, fileUrl) + ]; + // 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}`, '', fixes.get(problem), ''); + for (const {source, destination, line, detail} of entries) { + const where = line + ? linkTo(`line ${line}`, fileUrl && `${fileUrl}#L${line}`) + : 'entry'; + lines.push( + `- ${where}`, + ' ```ts', + ` source: '${source}',`, + ` destination: '${destination}'`, + ...(detail ? [` final destination: ${detail}`] : []), + ' ```' + ); + } + } + 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); +} + +main(); 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; 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",