Resolve internal links in the version being checked (#62754) · github/docs@9094ab9 · GitHub
Skip to content

Commit 9094ab9

Browse files
heiskrCopilot
heiskr
and
Copilot
authored
Resolve internal links in the version being checked (#62754)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 026fdb7b-a47f-4af1-bf87-caa90fbbdf7f
1 parent 5004215 commit 9094ab9

8 files changed

Lines changed: 419 additions & 19 deletions

File tree

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

Lines changed: 3 additions & 1 deletion

‎src/links/lib/extract-links.ts‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import path from 'path'
1111
import { createLogger } from '@/observability/logger'
1212
import { allVersions } from '@/versions/lib/all-versions'
1313
import { latestStable } from '@/versions/lib/enterprise-server-releases'
14+
import removeFPTFromPath from '@/versions/lib/remove-fpt-from-path'
1415
import { getDataByLanguage } from '@/data-directory/lib/get-data'
1516
import getRedirect from '@/redirects/lib/get-redirect'
1617
import { isArchivedVersionByPath } from '@/archives/lib/is-archived-version'
@@ -461,7 +462,12 @@ export function normalizeLinkPath(href: string): string {
461462
* target page's precomputed heading IDs. Redirects are intentionally excluded: the
462463
* link is already reported as a redirect-to-update, and its final anchor is ambiguous.
463464
*/
464-
export function resolveInternalLinkKey(href: string, pageMap: Record<string, Page>): string | null {
465+
export function resolveInternalLinkKey(
466+
href: string,
467+
pageMap: Record<string, Page>,
468+
version?: string,
469+
language = 'en',
470+
): string | null {
465471
const normalized = normalizeLinkPath(href)
466472

467473
const latestPrefix = '/enterprise-server@latest'
@@ -473,19 +479,62 @@ export function resolveInternalLinkKey(href: string, pageMap: Record<string, Pag
473479

474480
if (pageMap[resolved]) return resolved
475481

476-
const withLang = `/en${resolved}`
482+
// Try the version being checked before the bare `/en` fallback, matching the order
483+
// `checkInternalLink` uses. A page that applies to both FPT and GHES has a key for
484+
// each, and the `/en` key would otherwise win even during a GHES run. The heading
485+
// cache is keyed by that version's permalink, so the anchor would then be looked up
486+
// under the wrong key and silently skipped.
487+
const versioned = versionedPageKey(resolved, version, language)
488+
if (versioned && pageMap[versioned.key]) return versioned.key
489+
490+
const withLang = `/${language}${resolved}`
477491
if (pageMap[withLang]) return withLang
478492

479493
return null
480494
}
481495

496+
/**
497+
* Build the pageMap key for a versionless path inside a specific version, or return
498+
* null when the path shouldn't be resolved that way.
499+
*
500+
* pageMap keys look like `/en/enterprise-server@3.21/admin/foo`, with FPT omitting the
501+
* version segment entirely. A path that already carries its own version or language
502+
* prefix is left alone: it means something specific and shouldn't be reinterpreted as
503+
* relative to the version being checked.
504+
*
505+
* Returns the language-prefixed key for pageMap lookups and the language-stripped form,
506+
* which is how the redirect table stores paths.
507+
*/
508+
function versionedPageKey(
509+
resolved: string,
510+
version: string | undefined,
511+
language: string,
512+
): { key: string; withoutLanguage: string } | null {
513+
if (!version) return null
514+
// Already version-qualified, e.g. /enterprise-cloud@latest/..., so not a versionless link.
515+
if (/^\/[a-z-]+@/.test(resolved)) return null
516+
// Already language-qualified, e.g. /en/..., handled by the direct pageMap lookup above.
517+
if (/^\/[a-z]{2}(-[a-z]{2})?(\/|$)/.test(resolved)) return null
518+
519+
const withoutLanguage = removeFPTFromPath(path.posix.join('/', version, resolved))
520+
return { key: `/${language}${withoutLanguage}`, withoutLanguage }
521+
}
522+
482523
/**
483524
* Check if a path exists in the pageMap or redirects
525+
*
526+
* When `version` is supplied, a versionless link is first resolved inside that version,
527+
* mirroring what the runtime does when rendering the page. Without it, a versionless
528+
* link on a non-FPT page never matches a permalink (only FPT pages have versionless
529+
* permalinks), so it falls through to the versionless-fallback redirect and gets
530+
* misreported as a link that needs updating.
484531
*/
485532
export function checkInternalLink(
486533
href: string,
487534
pageMap: Record<string, Page>,
488535
redirects: Record<string, string>,
536+
version?: string,
537+
language = 'en',
489538
): { exists: boolean; isRedirect: boolean; redirectTarget?: string } {
490539
const normalized = normalizeLinkPath(href)
491540

@@ -503,6 +552,24 @@ export function checkInternalLink(
503552
return { exists: true, isRedirect: false }
504553
}
505554

555+
// A versionless link resolves within the version currently being checked. This has to
556+
// come before the redirect lookups on the versionless form: that form exists in the
557+
// redirect table as a fallback and would otherwise shadow a page that really exists
558+
// in this version.
559+
const versioned = versionedPageKey(resolved, version, language)
560+
if (versioned) {
561+
// Mirror runtime precedence: the redirect middleware runs before a page is served,
562+
// so a redirect on the effective versioned URL wins over the page itself. A
563+
// self-redirect is a no-op and doesn't count.
564+
const versionedRedirect = redirects[versioned.withoutLanguage]
565+
if (versionedRedirect && versionedRedirect !== versioned.withoutLanguage) {
566+
return { exists: true, isRedirect: true, redirectTarget: versionedRedirect }
567+
}
568+
if (pageMap[versioned.key]) {
569+
return { exists: true, isRedirect: false }
570+
}
571+
}
572+
506573
// Check if it's a redirect
507574
if (redirects[resolved]) {
508575
return {

‎src/links/lib/link-report.ts‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,20 @@ function groupByTarget(links: BrokenLink[]): Map<string, BrokenLink[]> {
218218
return groups
219219
}
220220

221+
const VERSION_PREFIX_RE = /^\/[a-z-]+@[^/]+/
222+
223+
/**
224+
* True when a redirect target is the same path with a version prefix bolted on.
225+
*
226+
* These aren't renames, they're the versionless link resolving into a version. Telling
227+
* an author to "update to the new path" here is actively wrong: hardcoding
228+
* `/enterprise-server@3.21/...` into content breaks as soon as 3.22 ships.
229+
*/
230+
function isVersionOnlyRedirect(target: string, redirectTarget: string): boolean {
231+
const withoutVersion = redirectTarget.replace(VERSION_PREFIX_RE, '')
232+
return withoutVersion === target
233+
}
234+
221235
/**
222236
* Create a suggestion message for a redirect
223237
*/
@@ -226,13 +240,33 @@ function createRedirectSuggestion(
226240
occurrences: BrokenLink[],
227241
redirects?: Record<string, string>,
228242
): string | undefined {
229-
if (redirects?.[target]) {
230-
return `This path redirects to \`${redirects[target]}\`. Consider updating to the new path.`
243+
const redirectTarget = redirects?.[target] ?? occurrences[0]?.redirectTarget
244+
if (!redirectTarget) return undefined
245+
246+
if (isVersionOnlyRedirect(target, redirectTarget)) {
247+
return (
248+
`This path resolves to \`${redirectTarget}\` in this version. Leave the link versionless: ` +
249+
`hardcoding a version breaks when the next release ships. If it should point at a ` +
250+
`different version, use a Liquid \`ifversion\` gate.`
251+
)
231252
}
232-
if (occurrences[0]?.redirectTarget) {
233-
return `This path redirects to \`${occurrences[0].redirectTarget}\`. Consider updating to the new path.`
253+
254+
// A versionless link that lands on a versioned path is a rename plus the version the
255+
// check happened to run in. Only the rename is real. Suggesting the target verbatim
256+
// would bake `enterprise-server@3.21` into content that never asked for a version.
257+
const sourceIsVersionless = !VERSION_PREFIX_RE.test(target)
258+
const versionPrefix = redirectTarget.match(VERSION_PREFIX_RE)?.[0]
259+
if (sourceIsVersionless && versionPrefix) {
260+
const withoutVersion = redirectTarget.slice(versionPrefix.length)
261+
return (
262+
`This path redirects to \`${withoutVersion}\`. Update the path but keep the link ` +
263+
`versionless: the \`${versionPrefix.slice(1)}\` prefix comes from the version being ` +
264+
`checked, not from the rename. Gate it with Liquid \`ifversion\` only if the new page ` +
265+
`really is version-specific.`
266+
)
234267
}
235-
return undefined
268+
269+
return `This path redirects to \`${redirectTarget}\`. Consider updating to the new path.`
236270
}
237271

238272
/**
@@ -345,8 +379,13 @@ export function generateInternalLinkReport(
345379
const errors = groups.filter((g) => !g.isWarning)
346380
const warnings = groups.filter((g) => g.isWarning)
347381

382+
// The workflow concatenates every version's report into one issue, so without this
383+
// label there's no way to tell which version a section covers.
384+
const scope = [options.version, options.language].filter(Boolean).join(' ')
385+
const scopeLabel = scope ? ` (${scope})` : ''
386+
348387
return {
349-
title: `Internal Link Check: ${errors.length} broken, ${warnings.length} redirects`,
388+
title: `Internal Link Check${scopeLabel}: ${errors.length} broken, ${warnings.length} redirects`,
350389
summary: createSummary(errors.length, warnings.length, brokenLinks.length),
351390
groups,
352391
uniqueTargets: groups.length,

‎src/links/lib/page-anchors.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,18 +61,20 @@ export function findLinkLines(content: string, hrefWithFragment: string): number
6161
* Resolve a link href to a pageMap key, considering the version the source page is being
6262
* rendered in.
6363
*
64-
* `resolveInternalLinkKey` only tries the href as written. An unversioned href like
65-
* `/copilot/foo` written on a GHEC-only page has no `/en/copilot/foo` key in the pageMap
66-
* (that key only exists when the target applies to FPT), so resolution returns null and
67-
* the link is silently skipped. Retrying with the source version prefixed picks up those
68-
* targets so their anchors get checked too.
64+
* `resolveInternalLinkKey` handles the common shapes once it knows the version, so hand
65+
* the version to it directly. That also fixes the precedence: a target that applies to
66+
* both FPT and the source version has a key for each, and without the version the `/en`
67+
* key wins even during an enterprise run.
68+
*
69+
* The explicit retry below still earns its place for hrefs that carry a language prefix,
70+
* which `resolveInternalLinkKey` refuses to reinterpret as relative to a version.
6971
*/
7072
export function resolveLinkKeyForVersion(
7173
href: string,
7274
version: string,
7375
pageMap: Record<string, Page>,
7476
): string | null {
75-
const direct = resolveInternalLinkKey(href, pageMap)
77+
const direct = resolveInternalLinkKey(href, pageMap, version)
7678
if (direct) return direct
7779

7880
// Only worth retrying when the href carries no version of its own.

‎src/links/scripts/check-links-internal.ts‎

Lines changed: 19 additions & 4 deletions

0 commit comments

Comments
 (0)