Skip to content

Commit 5e93b4b

Browse files
heiskrCopilot
andauthored
Add non-blocking cross-page anchor guardrail to the PR link check (#62417)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7bc51b35-b5bd-4cec-9948-a1664d171f4c Copilot-Session: 3993852f-89f9-401f-a2f8-2753a3d0ad43
1 parent eb2be54 commit 5e93b4b

6 files changed

Lines changed: 626 additions & 16 deletions

File tree

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ jobs:
5555
ACTION_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
5656
SHOULD_COMMENT: ${{ secrets.DOCS_BOT_PAT_BASE != '' }}
5757
FAIL_ON_FLAW: true
58+
# Cross-page anchor checking is on, but non-blocking during rollout: broken
59+
# anchors are reported in the PR comment without failing the build. Flip
60+
# FAIL_ON_ANCHOR_FLAW to true once false-positive/perf rates look clean.
61+
CHECK_ANCHORS: true
62+
FAIL_ON_ANCHOR_FLAW: false
5863
ENABLED_LANGUAGES: en
5964
run: npm run check-links-pr
6065

src/links/lib/link-report.ts

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ export interface BrokenLink {
2121
errorMessage?: string
2222
}
2323

24+
/**
25+
* A cross-page anchor link (`/path#fragment`) whose fragment doesn't match any heading on
26+
* the target page, along with the versions it breaks in. Reported separately from broken
27+
* page links because the target page exists — only the fragment is stale.
28+
*/
29+
export interface CrossPageAnchorFlaw {
30+
href: string
31+
file: string
32+
lines: number[]
33+
text?: string
34+
versions: string[]
35+
}
36+
2437
export interface GroupedBrokenLinks {
2538
target: string
2639
occurrences: BrokenLink[]
@@ -120,7 +133,12 @@ ${rows}`
120133
noIssues: () => 'No issues found! 🎉',
121134

122135
// PR comment
123-
prComment: (errors: GroupedBrokenLinks[], warnings: GroupedBrokenLinks[], actionUrl?: string) => {
136+
prComment: (
137+
errors: GroupedBrokenLinks[],
138+
warnings: GroupedBrokenLinks[],
139+
anchorSection: string,
140+
actionUrl?: string,
141+
) => {
124142
const errorSection =
125143
errors.length > 0
126144
? `### ⚠️ ${errors.length} Broken Link${errors.length === 1 ? '' : 's'}
@@ -151,9 +169,34 @@ ${errors
151169

152170
return `## 🔗 Link Check Results
153171
154-
${errorSection}${warningSection}${detailsLink}
172+
${errorSection}${warningSection}${anchorSection}${detailsLink}
155173
<!-- link-checker-pr-comment -->`
156174
},
175+
176+
// Cross-page anchor section. `blocking` reflects FAIL_ON_ANCHOR_FLAW so the wording
177+
// can't claim the check is advisory once the rollout flips it to failing.
178+
anchorSection: (anchors: CrossPageAnchorFlaw[], blocking = false) => {
179+
if (anchors.length === 0) return ''
180+
const shown = anchors.slice(0, 10)
181+
const remaining = anchors.length - shown.length
182+
const rows = shown
183+
.map(
184+
(a) =>
185+
`- \`${a.href}\`\n - \`${a.file}\` line ${a.lines.join(', ')} (${a.versions.join(', ')})`,
186+
)
187+
.join('\n')
188+
const moreLine = remaining > 0 ? `\n- ... and ${remaining} more` : ''
189+
const status = blocking
190+
? 'This check is failing.'
191+
: 'Not blocking yet, so this check still passes.'
192+
return `### ⚓ ${anchors.length} broken cross-page anchor${anchors.length === 1 ? '' : 's'}
193+
194+
These links resolve to a real page, but the \`#fragment\` no longer matches a heading on the target. ${status}
195+
196+
${rows}${moreLine}
197+
198+
`
199+
},
157200
}
158201

159202
// ============================================================================
@@ -422,15 +465,21 @@ export function reportToMarkdown(report: LinkReport, isExternal = false): string
422465
*/
423466
export function generatePRComment(
424467
brokenLinks: BrokenLink[],
425-
options: { actionUrl?: string } = {},
468+
options: {
469+
actionUrl?: string
470+
brokenAnchors?: CrossPageAnchorFlaw[]
471+
anchorsBlocking?: boolean
472+
} = {},
426473
): string {
427-
if (brokenLinks.length === 0) return ''
474+
const brokenAnchors = options.brokenAnchors ?? []
475+
if (brokenLinks.length === 0 && brokenAnchors.length === 0) return ''
428476

429477
const groups = groupBrokenLinks(brokenLinks)
430478
const errors = groups.filter((g) => !g.isWarning)
431479
const warnings = groups.filter((g) => g.isWarning)
480+
const anchorSection = TEMPLATES.anchorSection(brokenAnchors, options.anchorsBlocking)
432481

433-
return TEMPLATES.prComment(errors, warnings, options.actionUrl)
482+
return TEMPLATES.prComment(errors, warnings, anchorSection, options.actionUrl)
434483
}
435484

436485
// ============================================================================

src/links/lib/page-anchors.ts

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import type { Context, Page } from '@/types'
2+
import { allVersions } from '@/versions/lib/all-versions'
3+
import { getFeaturesByVersion } from '@/versions/middleware/features'
4+
import { getDataByLanguage, getDeepDataByLanguage } from '@/data-directory/lib/get-data'
5+
import {
6+
normalizeLinkPath,
7+
renderAndExtractLinks,
8+
resolveInternalLinkKey,
9+
} from '@/links/lib/extract-links'
10+
import { computeHeadingIds } from '@/links/lib/heading-anchors'
11+
12+
/**
13+
* Shared, version-aware helpers for validating that a `path#fragment` link lands on a
14+
* real heading of its target page. Kept neutral (no PR/CI specifics) so both the
15+
* scheduled internal checker and the PR-time gate can render a target page in a given
16+
* version and compute its heading anchor IDs the same way.
17+
*/
18+
19+
// Explicit version prefix on a resolved pageMap key, e.g. the `enterprise-cloud@latest`
20+
// in `/en/enterprise-cloud@latest/actions/foo`. `resolveInternalLinkKey` already maps
21+
// `enterprise-server@latest` to the stable release, so `@latest` never appears for GHES.
22+
const EXPLICIT_VERSION_RE =
23+
/^\/(?:[a-z]{2}(?:-[a-z]{2})?\/)?(free-pro-team@latest|enterprise-cloud@latest|enterprise-server@[0-9.]+)(?:\/|$)/
24+
25+
/**
26+
* Extract the version a resolved link key pins to, or null when the key carries no
27+
* explicit version segment (an unversioned link that inherits the source page's version).
28+
*/
29+
export function versionFromResolvedKey(key: string): string | null {
30+
const match = key.match(EXPLICIT_VERSION_RE)
31+
if (!match) return null
32+
return allVersions[match[1]] ? match[1] : null
33+
}
34+
35+
/**
36+
* Line numbers (1-based) in `content` where a Markdown link points at exactly
37+
* `hrefWithFragment`.
38+
*
39+
* The destination must END at the match: a bare substring search is a prefix match, so
40+
* looking for `](/a#foo` would also hit `](/a#foobar)` and misreport that line. A Markdown
41+
* destination ends at `)` or at whitespace before an optional title.
42+
*/
43+
export function findLinkLines(content: string, hrefWithFragment: string): number[] {
44+
const needle = `](${hrefWithFragment}`
45+
const lines = content.split('\n')
46+
const found: number[] = []
47+
for (let i = 0; i < lines.length; i++) {
48+
const line = lines[i]
49+
for (let at = line.indexOf(needle); at !== -1; at = line.indexOf(needle, at + 1)) {
50+
const next = line[at + needle.length]
51+
if (next === undefined || next === ')' || /\s/.test(next)) {
52+
found.push(i + 1)
53+
break
54+
}
55+
}
56+
}
57+
return found
58+
}
59+
60+
/**
61+
* Resolve a link href to a pageMap key, considering the version the source page is being
62+
* rendered in.
63+
*
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.
69+
*/
70+
export function resolveLinkKeyForVersion(
71+
href: string,
72+
version: string,
73+
pageMap: Record<string, Page>,
74+
): string | null {
75+
const direct = resolveInternalLinkKey(href, pageMap)
76+
if (direct) return direct
77+
78+
// Only worth retrying when the href carries no version of its own.
79+
const normalized = normalizeLinkPath(href)
80+
if (EXPLICIT_VERSION_RE.test(normalized)) return null
81+
82+
const withoutLang = normalized.replace(/^\/[a-z]{2}(?:-[a-z]{2})?(?=\/|$)/, '')
83+
return resolveInternalLinkKey(`/${version}${withoutLang}`, pageMap)
84+
}
85+
86+
/**
87+
* Whether a target page's heading anchors can be derived from its Markdown headings.
88+
*
89+
* Two page kinds compute their headings from data at runtime, not from static Markdown
90+
* headings, so `computeHeadingIds` can't see them and would report false positives:
91+
* - `autogenerated` pages (REST/GraphQL/webhooks/audit-log-events): headings come from
92+
* OpenAPI operation IDs / action prefixes.
93+
* - the glossary page: headings come from a `{% for glossary in glossaries %}` loop over
94+
* `data/glossaries/external.yml`, which isn't populated in this lightweight render.
95+
*
96+
* Links into these pages are skipped rather than flagged.
97+
*/
98+
export function isAnchorCheckableTarget(page: Page): boolean {
99+
if ((page as unknown as { autogenerated?: unknown }).autogenerated) return false
100+
const markdown = page.markdown
101+
if (
102+
typeof markdown === 'string' &&
103+
/\{%-?\s*for\s+\w+\s+in\s+glossaries\s*-?%\}/.test(markdown)
104+
) {
105+
return false
106+
}
107+
return true
108+
}
109+
110+
// Loaded once and shared: getDeepDataByLanguage walks the whole data/tables tree, which is
111+
// far too expensive to repeat per page per version.
112+
let tablesCache: Record<string, unknown> | null = null
113+
function getTables(): Record<string, unknown> {
114+
if (!tablesCache) tablesCache = getDeepDataByLanguage('tables', 'en')
115+
return tablesCache
116+
}
117+
118+
/**
119+
* Build a minimal per-version Liquid context for rendering a page's Markdown. Mirrors the
120+
* context the scheduled internal checker uses (version flags + feature flags), plus the
121+
* `variables` site data so `{% data variables.* %}` in headings resolves, and the `tables`
122+
* data so pages that generate headings from a `{% for %}` loop over a reference table
123+
* (e.g. `content/copilot/reference/copilot-feature-matrix.md`) produce their real heading
124+
* text instead of a literal `{{ groupName }}`, which would otherwise false-positive.
125+
*/
126+
export function buildRenderContext(
127+
page: Page,
128+
version: string,
129+
pageMap: Record<string, Page>,
130+
redirects: Record<string, string>,
131+
): Context {
132+
const versionObj = allVersions[version]
133+
return {
134+
currentVersion: version,
135+
currentLanguage: 'en',
136+
currentVersionObj: versionObj,
137+
[versionObj.shortName]: true,
138+
site: getDataByLanguage('variables', 'en'),
139+
tables: getTables(),
140+
page,
141+
pages: pageMap,
142+
redirects,
143+
...getFeaturesByVersion(version),
144+
} as unknown as Context
145+
}
146+
147+
/**
148+
* Compute the set of heading anchor IDs for a page rendered in a specific version.
149+
* Results are memoized in the optional `cache` (keyed by version + relative path) so a
150+
* target referenced by many links is only rendered once per version.
151+
*/
152+
export async function getPageHeadingIds(
153+
page: Page,
154+
version: string,
155+
pageMap: Record<string, Page>,
156+
redirects: Record<string, string>,
157+
cache?: Map<string, Set<string>>,
158+
): Promise<Set<string>> {
159+
const cacheKey = `${version}|${page.relativePath}`
160+
const cached = cache?.get(cacheKey)
161+
if (cached) return cached
162+
163+
const context = buildRenderContext(page, version, pageMap, redirects)
164+
const { renderedMarkdown } = await renderAndExtractLinks(page.markdown, context)
165+
const ids = computeHeadingIds(renderedMarkdown)
166+
167+
cache?.set(cacheKey, ids)
168+
return ids
169+
}

0 commit comments

Comments
 (0)