Repo sync

.github/workflows/link-check-internal.yml

@@ -212,6 +212,8 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+      - uses: ./.github/actions/node-npm-setup
+
       - name: Download all artifacts
         uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0
         with:
@@ -221,23 +223,23 @@ jobs:
 
       - name: Combine reports
         id: combine
+        env:
+          ACTION_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+          # A version with no broken links uploads no report, so the files on disk undercount
+          # what was checked. Pass the matrix so the report can say "broken in all versions"
+          # and mean it.
+          MATRIX: ${{ needs.setup-matrix.outputs.matrix }}
         run: |
-          # Check if any reports exist
-          if ls reports/*.md 1> /dev/null 2>&1; then
+          # Merge the per-version JSON rather than concatenating the rendered Markdown.
+          # A link broken in every version is one problem, not one per version.
+          if ls reports/*.json 1> /dev/null 2>&1; then
             echo "has_reports=true" >> $GITHUB_OUTPUT
-
-            # Combine all markdown reports
-            echo "# Internal Links Report" > combined-report.md
-            echo "" >> combined-report.md
-            echo "Generated: $(date -u +'%Y-%m-%d %H:%M UTC')" >> combined-report.md
-            echo "[Action run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})" >> combined-report.md
-            echo "" >> combined-report.md
-
-            for report in reports/*.md; do
-              echo "---" >> combined-report.md
-              cat "$report" >> combined-report.md
-              echo "" >> combined-report.md
-            done
+            VERSIONS=$(echo "$MATRIX" | jq -r '[.include[] | "\(.version) \(.language)"] | join(",")')
+            npm run combine-link-reports -- \
+              --input reports \
+              --output combined-report.md \
+              --versions "$VERSIONS" \
+              --action-url "$ACTION_RUN_URL"
           else
             echo "has_reports=false" >> $GITHUB_OUTPUT
             echo "No broken link reports generated - all links valid!"

package.json

@@ -91,6 +91,7 @@
     "liquid-tags": "tsx src/content-render/scripts/liquid-tags.ts",
     "check-links-pr": "tsx src/links/scripts/check-links-pr.ts",
     "check-links-internal": "tsx src/links/scripts/check-links-internal.ts",
+    "combine-link-reports": "tsx src/links/scripts/combine-link-reports.ts",
     "check-links-external": "tsx src/links/scripts/check-links-external.ts",
     "rest-dev": "tsx src/rest/scripts/update-files.ts",
     "show-action-deps": "echo 'Action Dependencies:' && rg '^[\\s|-]*(uses:.*)$' .github -I -N --no-heading -r '$1$2' | sort | uniq | cut -c 7-",

src/links/lib/link-report.ts

@@ -22,8 +22,20 @@ export interface BrokenLink {
    * `update-internal-links` looks the href up exactly as written, so it can't fix these.
    */
   requiresVersionContext?: boolean
+  /**
+   * Two checked versions resolved this href to genuinely different destinations, so no
+   * single rewrite is correct for all of them. Merging keeps one target and drops the
+   * rest, which would otherwise let the report name a destination that is only right for
+   * one version.
+   */
+  hasConflictingRedirectTargets?: boolean
   statusCode?: number
   errorMessage?: string
+  /**
+   * The versions this link is broken in. Only set on a merged report, where the same link
+   * usually breaks in every version checked.
+   */
+  versions?: string[]
 }
 
 /**
@@ -55,6 +67,8 @@ export interface LinkReport {
   totalOccurrences: number
   timestamp: string
   actionUrl?: string
+  /** Every version this report covers. Only set on a merged report. */
+  versionsChecked?: string[]
 }
 
 // ============================================================================
@@ -237,6 +251,15 @@ function isVersionOnlyRedirect(target: string, redirectTarget: string): boolean
   return withoutVersion === target
 }
 
+/**
+ * Two redirect targets that differ only by version prefix are the same rename seen from
+ * two versions, not a disagreement. `/enterprise-server@3.21/new` and
+ * `/enterprise-server@3.17/new` both mean "the page moved to /new".
+ */
+function sameDestination(a: string, b: string): boolean {
+  return a.replace(VERSION_PREFIX_RE, '') === b.replace(VERSION_PREFIX_RE, '')
+}
+
 /**
  * Create a suggestion message for a redirect
  */
@@ -368,6 +391,84 @@ function createSummary(errorCount: number, warningCount: number, totalOccurrence
   return `Found ${parts.join(' and ')} across ${totalOccurrences} occurrence${plural}.`
 }
 
+/**
+ * Describe which versions a link breaks in, but only when that is news.
+ *
+ * Nearly every broken link breaks in every version, so printing the full list on every
+ * group is noise that also blows past the issue body size limit. Say something only when a
+ * link is version-specific.
+ */
+export function describeVersions(
+  versions: string[] | undefined,
+  versionsChecked: string[] | undefined,
+): string | undefined {
+  if (!versions?.length || !versionsChecked?.length) return undefined
+  if (versionsChecked.length === 1) return undefined
+  if (versions.length >= versionsChecked.length) return undefined
+  return versions.join(', ')
+}
+
+/**
+ * Merge one report per version into a single report.
+ *
+ * The workflow used to concatenate each version's rendered Markdown, so a link broken in
+ * every version produced an identical section per version. Merging on the link itself means
+ * one section per real problem, with the versions recorded on the occurrence.
+ */
+export function mergeInternalLinkReports(
+  reports: { version: string; report: LinkReport }[],
+  options: { actionUrl?: string; versionsChecked?: string[] } = {},
+): LinkReport {
+  const merged = new Map<string, BrokenLink>()
+
+  for (const { version, report } of reports) {
+    for (const group of report.groups) {
+      for (const occurrence of group.occurrences) {
+        const href = occurrence.href || group.target
+        const key = `${href}\u0000${occurrence.file}`
+        const existing = merged.get(key)
+        if (existing) {
+          existing.lines = [...new Set([...existing.lines, ...occurrence.lines])].sort(
+            (a, b) => a - b,
+          )
+          existing.versions = [...new Set([...(existing.versions ?? []), version])]
+          // A link that redirects in any version is still worth rewriting everywhere.
+          existing.isRedirect = existing.isRedirect || occurrence.isRedirect
+          existing.requiresVersionContext =
+            existing.requiresVersionContext || occurrence.requiresVersionContext
+          // Keeping the first target and dropping the rest is only safe while every
+          // version agrees on where the page went. Today they always do, but if that ever
+          // stops being true the report would confidently name a destination that is
+          // right for one version and wrong for the others. Flag it instead.
+          if (
+            existing.redirectTarget &&
+            occurrence.redirectTarget &&
+            !sameDestination(existing.redirectTarget, occurrence.redirectTarget)
+          ) {
+            existing.hasConflictingRedirectTargets = true
+          }
+          existing.redirectTarget = existing.redirectTarget ?? occurrence.redirectTarget
+        } else {
+          merged.set(key, { ...occurrence, href, versions: [version] })
+        }
+      }
+    }
+  }
+
+  // A version with no broken links writes no report, so the files on disk undercount what
+  // was actually checked. Callers that know the full matrix pass it in, otherwise fall back
+  // to what was found.
+  const versionsChecked = options.versionsChecked?.length
+    ? options.versionsChecked
+    : reports.map((r) => r.version)
+  const report = generateInternalLinkReport([...merged.values()], options)
+  const scope =
+    versionsChecked.length > 1
+      ? `\n\nChecked ${versionsChecked.length} versions: ${versionsChecked.join(', ')}. A link listed without a version breaks in all of them.`
+      : ''
+  return { ...report, versionsChecked, summary: report.summary + scope }
+}
+
 /**
  * Generate a report for internal links
  */
@@ -472,6 +573,10 @@ export function classifyFixStrategy(group: GroupedBrokenLinks): FixStrategy {
     if (group.occurrences.some((occ) => occ.requiresVersionContext)) {
       return 'decide'
     }
+    // Versions disagree about where the page went, so there is no single correct rewrite.
+    if (group.occurrences.some((occ) => occ.hasConflictingRedirectTargets)) {
+      return 'decide'
+    }
     return 'codemod'
   }
   // The link carries a fragment, so the stale part is likely a renamed heading.
@@ -501,15 +606,30 @@ function codemodPaths(groups: GroupedBrokenLinks[]): string[] {
   return [...paths].sort()
 }
 
+/** The union of versions across a group's occurrences. */
+function groupVersions(group: GroupedBrokenLinks): string[] {
+  const versions = new Set<string>()
+  for (const occ of group.occurrences) {
+    for (const version of occ.versions ?? []) versions.add(version)
+  }
+  return [...versions]
+}
+
 function occurrenceCount(groups: GroupedBrokenLinks[]): number {
   return groups.reduce((sum, g) => sum + g.occurrences.length, 0)
 }
 
-function renderCodemodSection(groups: GroupedBrokenLinks[]): string {
+function renderCodemodSection(groups: GroupedBrokenLinks[], versionsChecked?: string[]): string {
+  const versionFor = (group: GroupedBrokenLinks) =>
+    describeVersions(groupVersions(group), versionsChecked)
+  const showVersions = groups.some((group) => versionFor(group))
+
   const rows = groups
     .map((group) => {
       const target = group.occurrences.find((occ) => occ.redirectTarget)?.redirectTarget ?? ''
-      return `| \`${group.target}\` | \`${target}\` | ${group.occurrences.length} |`
+      const cells = [`\`${group.target}\``, `\`${target}\``, `${group.occurrences.length}`]
+      if (showVersions) cells.push(versionFor(group) ?? 'all')
+      return `| ${cells.join(' | ')} |`
     })
     .join('\n')
 
@@ -542,8 +662,8 @@ Review the diff, then open a pull request.
 <details>
 <summary>The ${groups.length} link${plural} this fixes</summary>
 
-| From | To | Occurrences |
-|------|-----|-------------|
+| From | To | Occurrences |${showVersions ? ' Versions |' : ''}
+|------|-----|-------------|${showVersions ? '----------|' : ''}
 ${rows}
 
 </details>`
@@ -591,8 +711,15 @@ function renderManualSection(
   blurb: string,
   groups: GroupedBrokenLinks[],
   isExternal: boolean,
+  versionsChecked?: string[],
 ): string {
-  const sections = groups.map((group) => TEMPLATES.group(group, isExternal)).join('\n\n')
+  const sections = groups
+    .map((group) => {
+      const versions = describeVersions(groupVersions(group), versionsChecked)
+      const note = versions ? `\n\n**Only in:** ${versions}` : ''
+      return TEMPLATES.group(group, isExternal) + note
+    })
+    .join('\n\n')
   return `## ${heading} (${groups.length} link${groups.length === 1 ? '' : 's'}, ${occurrenceCount(groups)} occurrence${occurrenceCount(groups) === 1 ? '' : 's'})
 
 ${blurb}
@@ -604,7 +731,11 @@ ${sections}`
  * Render an internal report as four buckets ordered by how much work each one costs, from
  * one command down to nothing at all.
  */
-function renderByFixStrategy(groups: GroupedBrokenLinks[], isExternal: boolean): string {
+function renderByFixStrategy(
+  groups: GroupedBrokenLinks[],
+  isExternal: boolean,
+  versionsChecked?: string[],
+): string {
   const codemod = groups.filter((g) => classifyFixStrategy(g) === 'codemod')
   const versionless = groups.filter((g) => classifyFixStrategy(g) === 'versionless')
   const anchors = groups.filter((g) => classifyFixStrategy(g) === 'anchor')
@@ -631,14 +762,15 @@ ${summaryRows.join('\n')}
 Work top to bottom. Bucket 1 is usually most of the report and costs one command.`,
   ]
 
-  if (codemod.length > 0) parts.push(renderCodemodSection(codemod))
+  if (codemod.length > 0) parts.push(renderCodemodSection(codemod, versionsChecked))
   if (anchors.length > 0) {
     parts.push(
       renderManualSection(
         '2. Stale anchors',
         'The `#fragment` does not match a heading on the target page. Usually a heading was renamed: find it and repoint the link, or drop the fragment if the section is gone. Check that the page itself still exists first, since a missing page with a fragment also lands here.',
         anchors,
         isExternal,
+        versionsChecked,
       ),
     )
   }
@@ -649,6 +781,7 @@ Work top to bottom. Bucket 1 is usually most of the report and costs one command
         'The codemod looks each link up exactly as written, and for these that lookup finds nothing: either no redirect exists at all, or the redirect only exists under a version prefix the link does not carry. Choose a destination, or add a redirect from the path as written.',
         decide,
         isExternal,
+        versionsChecked,
       ),
     )
   }
@@ -725,7 +858,7 @@ export function reportToMarkdown(report: LinkReport, isExternal = false): string
     parts.push(
       isExternal
         ? renderGroups(report.groups, isExternal)
-        : renderByFixStrategy(report.groups, isExternal),
+        : renderByFixStrategy(report.groups, isExternal, report.versionsChecked),
     )
   }
 

src/links/scripts/combine-link-reports.ts

@@ -0,0 +1,79 @@
+#!/usr/bin/env tsx
+
+/**
+ * Combine every version's link report into one deduplicated Markdown report.
+ *
+ * The workflow used to `cat` each version's rendered Markdown together, so a link broken in
+ * every version produced an identical section per version. That multiplied the report by the
+ * size of the matrix and pushed it past the issue body limit, where it got truncated.
+ */
+
+import fs from 'fs'
+import path from 'path'
+import { program } from 'commander'
+
+import {
+  mergeInternalLinkReports,
+  reportToMarkdown,
+  type LinkReport,
+} from '@/links/lib/link-report'
+
+// `link-report-free-pro-team@latest-en.json` -> `free-pro-team@latest en`
+const REPORT_FILE = /^link-report-(.+)-([a-z]{2})\.json$/
+
+interface VersionedReport {
+  version: string
+  report: LinkReport
+}
+
+export function readReports(directory: string): VersionedReport[] {
+  if (!fs.existsSync(directory)) return []
+
+  const reports: VersionedReport[] = []
+  for (const file of fs.readdirSync(directory).sort()) {
+    const match = REPORT_FILE.exec(file)
+    if (!match) continue
+
+    const [, version, language] = match
+    const raw = fs.readFileSync(path.join(directory, file), 'utf8')
+    reports.push({ version: `${version} ${language}`, report: JSON.parse(raw) as LinkReport })
+  }
+  return reports
+}
+
+async function main() {
+  program
+    .description('Combine per-version link reports into one deduplicated report')
+    .option('-i, --input <directory>', 'Directory holding the report JSON files', 'reports')
+    .option('-o, --output <file>', 'Where to write the combined Markdown', 'combined-report.md')
+    .option('--action-url <url>', 'Link back to the workflow run')
+    .option(
+      '--versions <list>',
+      'Comma-separated list of every version checked, including the ones that came back clean',
+    )
+    .parse()
+
+  const { input, output, actionUrl, versions } = program.opts()
+  const versionsChecked = versions
+    ? String(versions)
+        .split(',')
+        .map((v: string) => v.trim())
+        .filter(Boolean)
+    : undefined
+  const reports = readReports(input)
+
+  if (reports.length === 0) {
+    console.log(`No report JSON found in ${input}.`)
+    process.exit(1)
+  }
+
+  const merged = mergeInternalLinkReports(reports, { actionUrl, versionsChecked })
+  fs.writeFileSync(output, reportToMarkdown(merged))
+
+  const before = reports.reduce((sum, r) => sum + r.report.groups.length, 0)
+  console.log(
+    `Combined ${reports.length} report(s): ${before} sections before, ${merged.groups.length} after.`,
+  )
+}
+
+main()