Make internal link checker blocking on PRs (#62268)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 031a122f-ce90-4f05-8641-f98155428219
Copilot-Session: 15ec74e5-6592-4b40-9482-74d62bd15d7c

Author: heiskr

.github/workflows/link-check-on-pr.yml

@@ -59,14 +59,12 @@ jobs:
 
       - name: Check links in changed files
         if: steps.changed-files.outputs.any_changed == 'true'
-        # Work in progress: never fail the PR. The comment is informational only.
-        continue-on-error: true
         env:
           FILES_CHANGED: ${{ steps.changed-files.outputs.all_changed_files }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           ACTION_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
           SHOULD_COMMENT: ${{ secrets.DOCS_BOT_APP_ID != '' }}
-          FAIL_ON_FLAW: false
+          FAIL_ON_FLAW: true
           ENABLED_LANGUAGES: en
         run: npm run check-links-pr
 

data/reusables/security-configurations/custom-security-configurations-org.md

@@ -1,2 +1 @@
-1. Create a new custom security configuration, or edit an existing one. See [AUTOTITLE](/code-security/how-tos/secure-at-scale/configure-organization-security/establish-complete-coverage/create-custom-configuration.
-
+1. Create a new custom security configuration, or edit an existing one. See [AUTOTITLE](/code-security/how-tos/secure-at-scale/configure-organization-security/establish-complete-coverage/create-custom-configuration).

src/links/lib/extract-links.ts

@@ -19,8 +19,8 @@ import type { Context, Page } from '@/types'
 const logger = createLogger(import.meta.url)
 
 // Link patterns for Markdown
-const INTERNAL_LINK_PATTERN = /\]\(\/[^)]+\)/g
-const AUTOTITLE_LINK_PATTERN = /\[AUTOTITLE\]\(([^)]+)\)/g
+const INTERNAL_LINK_PATTERN = /\]\((\/[^()\s]+(?:\([^()]*\)[^()\s]*)*)\)/g
+const AUTOTITLE_LINK_PATTERN = /\[AUTOTITLE\]\(([^)\s]+)\)/g
 // Handles one level of balanced parentheses in URLs (e.g., Wikipedia links).
 // Uses an unrolled loop to avoid catastrophic backtracking on malformed URLs.
 const EXTERNAL_LINK_PATTERN = /\]\((https?:\/\/[^()\s]*(?:\([^()]*\)[^()\s]*)*)\)/g
@@ -128,13 +128,27 @@ export function extractLinksFromMarkdown(content: string): LinkExtractionResult
 
   // Strip fenced code blocks to avoid checking example/placeholder URLs
   // Replaces non-newline characters with spaces to preserve line numbers and positions
-  const strippedContent = content.replace(
+  const withoutFences = content.replace(
     /^ {0,3}(`{3,})[^\n]*\n[\s\S]*?^ {0,3}\1\s*$/gm,
     (match) => {
       return match.replace(/[^\n]/g, ' ')
     },
   )
 
+  // Strip inline code spans too, so example or placeholder links written inside
+  // backticks (e.g. `[AUTOTITLE](/PATH/TO/PAGE)` in the style guide) aren't
+  // treated as real links. Markdown never renders links inside inline code.
+  //
+  // Per CommonMark, a code span opens with a backtick run of length N and closes
+  // with a run of exactly N backticks; both runs must be maximal, i.e. not
+  // adjacent to another backtick. The lookarounds enforce that so mismatched
+  // runs (e.g. `x`` or ``x```) stay literal instead of masking real text (and a
+  // real link) between them, which would let a broken link evade the check.
+  // The content is replaced with spaces to preserve line numbers and positions.
+  const strippedContent = withoutFences.replace(/(?<!`)(`+)(?!`)[^\n]*?(?<!`)\1(?!`)/g, (match) => {
+    return match.replace(/[^\n]/g, ' ')
+  })
+
   // Precompute line-start offsets once so every getLineAndColumn call is O(log L).
   const lineOffsets = buildLineOffsets(strippedContent)
 
@@ -160,14 +174,14 @@ export function extractLinksFromMarkdown(content: string): LinkExtractionResult
   // Extract regular internal links
   while ((match = INTERNAL_LINK_PATTERN.exec(strippedContent)) !== null) {
     // Skip if this is an AUTOTITLE link (already captured)
-    const fullMatch = match[0]
     if (strippedContent.substring(match.index - 10, match.index).includes('AUTOTITLE')) {
       continue
     }
 
     const { line, column } = getLineAndColumn(lineOffsets, match.index)
-    // Extract href from ](/path) format
-    const href = fullMatch.substring(2, fullMatch.length - 1).split('#')[0]
+    // Extract href from ](/path) format. The destination is captured in group 1,
+    // which handles balanced parentheses (e.g. asset filenames like `(fr).pdf`).
+    const href = match[1].split('#')[0]
     const text = extractLinkText(strippedContent, match.index)
 
     internalLinks.push({

src/links/lib/link-report.ts

@@ -151,9 +151,6 @@ ${errors
 
     return `## 🔗 Link Check Results
 
-> [!NOTE]
-> **This check is being actively worked on** and may produce false positives. If something looks wrong, you can safely ignore it for now.
-
 ${errorSection}${warningSection}${detailsLink}
 <!-- link-checker-pr-comment -->`
   },

src/links/scripts/check-links-pr.ts

@@ -150,6 +150,9 @@ function filterContentFiles(files: string[]): string[] {
   return files.filter((file) => {
     // Only check Markdown files in content/ or data/
     if (!file.endsWith('.md')) return false
+    // Skip README.md files. They're developer docs, not published pages, and use
+    // repo-relative paths (e.g. /src/...) that aren't valid site links.
+    if (file === 'README.md' || file.endsWith('/README.md')) return false
     if (file.startsWith('content/') || file.startsWith('data/')) return true
     return false
   })
@@ -338,15 +341,24 @@ async function main() {
     }
   }
 
-  // Write artifact for debugging
+  // Write artifact for debugging. Best-effort: a reporting/API failure must
+  // never fail the build. Only broken links (below) should fail the PR.
   const allFlaws = [...allBrokenLinks, ...allRedirectLinks]
-  await uploadArtifact('broken-links.json', JSON.stringify(groupBrokenLinks(allFlaws), null, 2))
+  try {
+    await uploadArtifact('broken-links.json', JSON.stringify(groupBrokenLinks(allFlaws), null, 2))
+  } catch (err) {
+    console.warn('Could not upload broken-links artifact:', err)
+  }
 
-  // Post PR comment if configured
+  // Post PR comment if configured. Best-effort for the same reason.
   const shouldComment = process.env.SHOULD_COMMENT === 'true'
   if (shouldComment) {
     const actionUrl = process.env.ACTION_RUN_URL
-    await commentOnPR(allFlaws, actionUrl)
+    try {
+      await commentOnPR(allFlaws, actionUrl)
+    } catch (err) {
+      console.warn('Could not post PR comment:', err)
+    }
   }
 
   // Exit with error if broken links found

src/links/tests/extract-links.ts

@@ -210,6 +210,74 @@ And [another real link](https://real.example.com/page).
     expect(result.externalLinks[1].href).toBe('https://real.example.com/page')
   })
 
+  test('skips links inside inline code spans', () => {
+    const content = `
+See [a real link](/real/path) for details.
+
+* For links to other pages: \`See [AUTOTITLE](/PATH/TO/PAGE).\`
+* With a query: \`See [AUTOTITLE](/path/to/page?tool=TOOLNAME).\`
+
+And [another real link](/another/real/path) here.
+`
+    const result = extractLinksFromMarkdown(content)
+
+    expect(result.internalLinks).toHaveLength(2)
+    expect(result.internalLinks.map((l) => l.href)).toEqual(['/real/path', '/another/real/path'])
+  })
+
+  test('handles inline code and a real link on the same line', () => {
+    const content = `Use \`[AUTOTITLE](/PLACEHOLDER)\` and then see [the guide](/real/guide).`
+    const result = extractLinksFromMarkdown(content)
+
+    expect(result.internalLinks).toHaveLength(1)
+    expect(result.internalLinks[0].href).toBe('/real/guide')
+  })
+
+  test('does not mask links when backtick runs are mismatched', () => {
+    // Per CommonMark, a code span needs equal-length, maximal backtick runs on
+    // both ends. These lines have mismatched runs, so they are NOT code spans
+    // and the links between the backticks are real and must be extracted.
+    const content = [
+      `A single-open, double-close: \`[one](/real/one)\`\``,
+      `A double-open, triple-close: \`\`[two](/real/two)\`\`\``,
+    ].join('\n')
+    const result = extractLinksFromMarkdown(content)
+
+    expect(result.internalLinks.map((l) => l.href)).toEqual(['/real/one', '/real/two'])
+  })
+
+  test('still masks links inside valid multi-backtick code spans', () => {
+    // A matched double-backtick run is a real code span, even when it wraps an
+    // inner single backtick, so the link inside must be ignored.
+    const content = `Example: \`\` \`[skip](/placeholder)\` \`\` and see [the guide](/real/guide).`
+    const result = extractLinksFromMarkdown(content)
+
+    expect(result.internalLinks).toHaveLength(1)
+    expect(result.internalLinks[0].href).toBe('/real/guide')
+  })
+
+  test('captures internal links with balanced parentheses in the path', () => {
+    const content = `See the [privacy statement (PDF)](/assets/images/help/site-policy/github-privacy-statement(07.22.20)(fr).pdf).`
+    const result = extractLinksFromMarkdown(content)
+
+    expect(result.internalLinks).toHaveLength(1)
+    expect(result.internalLinks[0].href).toBe(
+      '/assets/images/help/site-policy/github-privacy-statement(07.22.20)(fr).pdf',
+    )
+  })
+
+  test('does not let an unclosed link destination span multiple lines', () => {
+    const content = `
+Broken: [AUTOTITLE](/code-security/create-custom-configuration.
+1. A following list item with [a real link](/real/target).
+`
+    const result = extractLinksFromMarkdown(content)
+
+    // The unclosed link is not extracted, and it does not swallow the real link
+    // on the next line into a giant multi-line href.
+    expect(result.internalLinks.map((l) => l.href)).toEqual(['/real/target'])
+  })
+
   test('handles complex nested brackets', () => {
     const content = `
 Use the [\`git clone\`](/repositories/cloning) command.