diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml new file mode 100644 index 000000000..8c9af1d1b --- /dev/null +++ b/.github/workflows/check-links.yml @@ -0,0 +1,92 @@ +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) + + # 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/AGENTS.md b/AGENTS.md index 9a5e9b001..cdb3ac2a2 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`) +- **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..07ad7b0bd 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -8,106 +8,163 @@ * * 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'; 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 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; 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 -> / +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 }); + // 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); - // 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 }; -} - -// Check if a path exists in public directory -function checkPublicPath(linkPath) { - const publicPath = path.join(path.dirname(__dirname), 'public', linkPath); - return fs.existsSync(publicPath); + return { pathMap, routesByLowerCase, headingsMap, headingsByFile, assetsByLowerCase: await buildAssetMap() }; } -// 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 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 +194,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 +221,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 +265,153 @@ 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'); - - const { pathMap, headingsMap } = await buildPathMap(); - const files = await glob('**/*.mdx', { cwd: DOCS_DIR }); - - let totalErrors = 0; - const errors = []; +// 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 = []; - 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 => { - 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;