Skip to content

Commit 6d9d324

Browse files
heiskrCopilot
andauthored
Fix internal link checker false positives via runtime redirect resolver (#62098)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2ced51d commit 6d9d324

2 files changed

Lines changed: 106 additions & 2 deletions

File tree

src/links/lib/extract-links.ts

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { createLogger } from '@/observability/logger'
1212
import { allVersions } from '@/versions/lib/all-versions'
1313
import { latestStable } from '@/versions/lib/enterprise-server-releases'
1414
import { getDataByLanguage } from '@/data-directory/lib/get-data'
15+
import getRedirect from '@/redirects/lib/get-redirect'
16+
import { isArchivedVersionByPath } from '@/archives/lib/is-archived-version'
1517
import type { Context, Page } from '@/types'
1618

1719
const logger = createLogger(import.meta.url)
@@ -447,8 +449,10 @@ export function checkInternalLink(
447449
}
448450
}
449451

450-
// Strip language prefix and check redirects (which are stored without it)
451-
const langPrefixMatch = resolved.match(/^\/[a-z]{2}\//)
452+
// Strip language prefix and check redirects (which are stored without it).
453+
// Match hyphenated locales too (e.g. /pt-br/, /zh-cn/) so we don't later
454+
// double-prefix them with /en.
455+
const langPrefixMatch = resolved.match(/^\/[a-z]{2}(-[a-z]{2})?\//)
452456
if (langPrefixMatch) {
453457
const withoutLang = resolved.slice(langPrefixMatch[0].length - 1)
454458
if (redirects[withoutLang]) {
@@ -460,6 +464,49 @@ export function checkInternalLink(
460464
}
461465
}
462466

467+
// The path in language-prefixed form, used by the runtime resolvers below.
468+
// Avoid double-prefixing when the link already carried a language code.
469+
const withEn = langPrefixMatch ? resolved : withLang
470+
471+
// Links into deprecated/archived Enterprise Server versions (e.g.
472+
// /enterprise-server@3.7/... or the legacy /enterprise/2.1/... format) are
473+
// served by the archived enterprise versions system, which isn't loaded into
474+
// pageMap. They resolve fine at runtime, so treat them as valid rather than
475+
// broken.
476+
if (isArchivedVersionByPath(withEn).isArchived) {
477+
return { exists: true, isRedirect: false }
478+
}
479+
480+
// Fall back to the runtime redirect resolver. It handles algorithmic
481+
// corrections (version-prefix normalization, /admin, /desktop/guides, etc.)
482+
// that the flat redirects map doesn't contain as literal keys. This mirrors
483+
// what the production server actually does, so a link that redirects in
484+
// production is reported as a redirect here instead of a false broken link.
485+
try {
486+
// Only redirects, userLanguage, and pages are read by getRedirect (and the
487+
// resolvers it delegates to), so type the object to those fields rather than
488+
// casting an arbitrary shape to the full Context.
489+
const context: Pick<Context, 'redirects' | 'userLanguage' | 'pages'> = {
490+
redirects,
491+
userLanguage: 'en',
492+
pages: pageMap,
493+
}
494+
const redirect = getRedirect(withEn, context as unknown as Context)
495+
if (redirect) {
496+
// getRedirect returns a language-prefixed path (e.g. /en/...); strip any
497+
// locale prefix to match the format used by the flat-map branches above,
498+
// and normalize a bare language root (e.g. /en) to /.
499+
return {
500+
exists: true,
501+
isRedirect: true,
502+
redirectTarget: redirect.replace(/^\/[a-z]{2}(-[a-z]{2})?(?=\/|$)/, '') || '/',
503+
}
504+
}
505+
} catch {
506+
// getRedirect throws on a few fully-deprecated shapes (e.g. github-ae).
507+
// Treat those as unresolvable rather than crashing the whole check.
508+
}
509+
463510
return { exists: false, isRedirect: false }
464511
}
465512

src/links/tests/extract-links.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,63 @@ describe('checkInternalLink', () => {
434434
expect(result.isRedirect).toBe(true)
435435
expect(result.redirectTarget).toBe('/actions/current-path')
436436
})
437+
438+
test('treats archived Enterprise Server versions as valid', () => {
439+
// Deprecated GHES versions are served by the archived enterprise versions
440+
// system, which isn't loaded into pageMap. They must not be reported broken.
441+
const result = checkInternalLink(
442+
'/enterprise-server@3.7/admin/release-notes',
443+
pageMap,
444+
redirects,
445+
)
446+
expect(result.exists).toBe(true)
447+
expect(result.isRedirect).toBe(false)
448+
})
449+
450+
test('treats legacy /enterprise/<version>/ archived paths as valid', () => {
451+
const result = checkInternalLink(
452+
'/enterprise/2.1/admin/guides/installation/provisioning-and-installation/',
453+
pageMap,
454+
redirects,
455+
)
456+
expect(result.exists).toBe(true)
457+
expect(result.isRedirect).toBe(false)
458+
})
459+
460+
test('resolves free-pro-team@latest prefixed links via the redirect resolver', () => {
461+
// The flat redirects map has no literal key for this; getRedirect computes
462+
// the correction (strip the version prefix) the same way production does.
463+
const result = checkInternalLink('/free-pro-team@latest/actions/guides', pageMap, redirects)
464+
expect(result.exists).toBe(true)
465+
expect(result.isRedirect).toBe(true)
466+
expect(result.redirectTarget).toBe('/actions/guides')
467+
})
468+
469+
test('resolves versionless /enterprise-server/ links via the redirect resolver', () => {
470+
const result = checkInternalLink(`/enterprise-server/admin/overview`, pageMap, redirects)
471+
expect(result.exists).toBe(true)
472+
expect(result.isRedirect).toBe(true)
473+
// Normalized to the latest stable Enterprise Server version.
474+
expect(result.redirectTarget).toBe(`/enterprise-server@${latestStable}/admin/overview`)
475+
})
476+
477+
test('strips hyphenated locale prefixes without double-prefixing', () => {
478+
// /pt-br/ is a hyphenated locale; it must be stripped (not turned into
479+
// /en/pt-br/...) so the underlying path resolves against the redirects map.
480+
const result = checkInternalLink('/pt-br/actions/legacy-path', pageMap, redirects)
481+
expect(result.exists).toBe(true)
482+
expect(result.isRedirect).toBe(true)
483+
expect(result.redirectTarget).toBe('/actions/current-path')
484+
})
485+
486+
test('normalizes a bare language-root redirect target to /', () => {
487+
// getRedirect collapses '/free-pro-team@latest' to the language root ('/en');
488+
// after stripping the locale that would be empty, so it must normalize to '/'.
489+
const result = checkInternalLink('/free-pro-team@latest', pageMap, redirects)
490+
expect(result.exists).toBe(true)
491+
expect(result.isRedirect).toBe(true)
492+
expect(result.redirectTarget).toBe('/')
493+
})
437494
})
438495

439496
describe('isAssetLink', () => {

0 commit comments

Comments
 (0)