From 9094ab960a8657944ae4786d9f93b99bb389bb63 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 17:42:13 +0000 Subject: [PATCH 1/8] 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 --- .github/workflows/link-check-internal.yml | 4 +- src/links/lib/extract-links.ts | 71 +++++++++- src/links/lib/link-report.ts | 51 ++++++- src/links/lib/page-anchors.ts | 14 +- src/links/scripts/check-links-internal.ts | 23 ++- src/links/tests/extract-links.ts | 164 ++++++++++++++++++++++ src/links/tests/link-report.ts | 91 ++++++++++++ src/links/tests/page-anchors.ts | 20 +++ 8 files changed, 419 insertions(+), 19 deletions(-) diff --git a/.github/workflows/link-check-internal.yml b/.github/workflows/link-check-internal.yml index 47d465e43ff4..5674853ff35a 100644 --- a/.github/workflows/link-check-internal.yml +++ b/.github/workflows/link-check-internal.yml @@ -103,7 +103,9 @@ jobs: uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: link-report-${{ matrix.version }}-${{ matrix.language }} - path: artifacts/link-report-*.md + # Include the JSON alongside the Markdown so the structured report is + # consumable outside this job, not just readable in the issue body. + path: artifacts/link-report-* retention-days: 5 if-no-files-found: ignore diff --git a/src/links/lib/extract-links.ts b/src/links/lib/extract-links.ts index 1d0db58e4a4f..6ac63609e8ec 100644 --- a/src/links/lib/extract-links.ts +++ b/src/links/lib/extract-links.ts @@ -11,6 +11,7 @@ import path from 'path' import { createLogger } from '@/observability/logger' import { allVersions } from '@/versions/lib/all-versions' import { latestStable } from '@/versions/lib/enterprise-server-releases' +import removeFPTFromPath from '@/versions/lib/remove-fpt-from-path' import { getDataByLanguage } from '@/data-directory/lib/get-data' import getRedirect from '@/redirects/lib/get-redirect' import { isArchivedVersionByPath } from '@/archives/lib/is-archived-version' @@ -461,7 +462,12 @@ export function normalizeLinkPath(href: string): string { * target page's precomputed heading IDs. Redirects are intentionally excluded: the * link is already reported as a redirect-to-update, and its final anchor is ambiguous. */ -export function resolveInternalLinkKey(href: string, pageMap: Record): string | null { +export function resolveInternalLinkKey( + href: string, + pageMap: Record, + version?: string, + language = 'en', +): string | null { const normalized = normalizeLinkPath(href) const latestPrefix = '/enterprise-server@latest' @@ -473,19 +479,62 @@ export function resolveInternalLinkKey(href: string, pageMap: Record, redirects: Record, + version?: string, + language = 'en', ): { exists: boolean; isRedirect: boolean; redirectTarget?: string } { const normalized = normalizeLinkPath(href) @@ -503,6 +552,24 @@ export function checkInternalLink( return { exists: true, isRedirect: false } } + // A versionless link resolves within the version currently being checked. This has to + // come before the redirect lookups on the versionless form: that form exists in the + // redirect table as a fallback and would otherwise shadow a page that really exists + // in this version. + const versioned = versionedPageKey(resolved, version, language) + if (versioned) { + // Mirror runtime precedence: the redirect middleware runs before a page is served, + // so a redirect on the effective versioned URL wins over the page itself. A + // self-redirect is a no-op and doesn't count. + const versionedRedirect = redirects[versioned.withoutLanguage] + if (versionedRedirect && versionedRedirect !== versioned.withoutLanguage) { + return { exists: true, isRedirect: true, redirectTarget: versionedRedirect } + } + if (pageMap[versioned.key]) { + return { exists: true, isRedirect: false } + } + } + // Check if it's a redirect if (redirects[resolved]) { return { diff --git a/src/links/lib/link-report.ts b/src/links/lib/link-report.ts index 6e045c357a34..69d6a8588eda 100644 --- a/src/links/lib/link-report.ts +++ b/src/links/lib/link-report.ts @@ -218,6 +218,20 @@ function groupByTarget(links: BrokenLink[]): Map { return groups } +const VERSION_PREFIX_RE = /^\/[a-z-]+@[^/]+/ + +/** + * True when a redirect target is the same path with a version prefix bolted on. + * + * These aren't renames, they're the versionless link resolving into a version. Telling + * an author to "update to the new path" here is actively wrong: hardcoding + * `/enterprise-server@3.21/...` into content breaks as soon as 3.22 ships. + */ +function isVersionOnlyRedirect(target: string, redirectTarget: string): boolean { + const withoutVersion = redirectTarget.replace(VERSION_PREFIX_RE, '') + return withoutVersion === target +} + /** * Create a suggestion message for a redirect */ @@ -226,13 +240,33 @@ function createRedirectSuggestion( occurrences: BrokenLink[], redirects?: Record, ): string | undefined { - if (redirects?.[target]) { - return `This path redirects to \`${redirects[target]}\`. Consider updating to the new path.` + const redirectTarget = redirects?.[target] ?? occurrences[0]?.redirectTarget + if (!redirectTarget) return undefined + + if (isVersionOnlyRedirect(target, redirectTarget)) { + return ( + `This path resolves to \`${redirectTarget}\` in this version. Leave the link versionless: ` + + `hardcoding a version breaks when the next release ships. If it should point at a ` + + `different version, use a Liquid \`ifversion\` gate.` + ) } - if (occurrences[0]?.redirectTarget) { - return `This path redirects to \`${occurrences[0].redirectTarget}\`. Consider updating to the new path.` + + // A versionless link that lands on a versioned path is a rename plus the version the + // check happened to run in. Only the rename is real. Suggesting the target verbatim + // would bake `enterprise-server@3.21` into content that never asked for a version. + const sourceIsVersionless = !VERSION_PREFIX_RE.test(target) + const versionPrefix = redirectTarget.match(VERSION_PREFIX_RE)?.[0] + if (sourceIsVersionless && versionPrefix) { + const withoutVersion = redirectTarget.slice(versionPrefix.length) + return ( + `This path redirects to \`${withoutVersion}\`. Update the path but keep the link ` + + `versionless: the \`${versionPrefix.slice(1)}\` prefix comes from the version being ` + + `checked, not from the rename. Gate it with Liquid \`ifversion\` only if the new page ` + + `really is version-specific.` + ) } - return undefined + + return `This path redirects to \`${redirectTarget}\`. Consider updating to the new path.` } /** @@ -345,8 +379,13 @@ export function generateInternalLinkReport( const errors = groups.filter((g) => !g.isWarning) const warnings = groups.filter((g) => g.isWarning) + // The workflow concatenates every version's report into one issue, so without this + // label there's no way to tell which version a section covers. + const scope = [options.version, options.language].filter(Boolean).join(' ') + const scopeLabel = scope ? ` (${scope})` : '' + return { - title: `Internal Link Check: ${errors.length} broken, ${warnings.length} redirects`, + title: `Internal Link Check${scopeLabel}: ${errors.length} broken, ${warnings.length} redirects`, summary: createSummary(errors.length, warnings.length, brokenLinks.length), groups, uniqueTargets: groups.length, diff --git a/src/links/lib/page-anchors.ts b/src/links/lib/page-anchors.ts index 9473b86d96c0..bd7738104228 100644 --- a/src/links/lib/page-anchors.ts +++ b/src/links/lib/page-anchors.ts @@ -61,18 +61,20 @@ export function findLinkLines(content: string, hrefWithFragment: string): number * Resolve a link href to a pageMap key, considering the version the source page is being * rendered in. * - * `resolveInternalLinkKey` only tries the href as written. An unversioned href like - * `/copilot/foo` written on a GHEC-only page has no `/en/copilot/foo` key in the pageMap - * (that key only exists when the target applies to FPT), so resolution returns null and - * the link is silently skipped. Retrying with the source version prefixed picks up those - * targets so their anchors get checked too. + * `resolveInternalLinkKey` handles the common shapes once it knows the version, so hand + * the version to it directly. That also fixes the precedence: a target that applies to + * both FPT and the source version has a key for each, and without the version the `/en` + * key wins even during an enterprise run. + * + * The explicit retry below still earns its place for hrefs that carry a language prefix, + * which `resolveInternalLinkKey` refuses to reinterpret as relative to a version. */ export function resolveLinkKeyForVersion( href: string, version: string, pageMap: Record, ): string | null { - const direct = resolveInternalLinkKey(href, pageMap) + const direct = resolveInternalLinkKey(href, pageMap, version) if (direct) return direct // Only worth retrying when the href carries no version of its own. diff --git a/src/links/scripts/check-links-internal.ts b/src/links/scripts/check-links-internal.ts index 26993356a473..9e0b234827d4 100644 --- a/src/links/scripts/check-links-internal.ts +++ b/src/links/scripts/check-links-internal.ts @@ -265,7 +265,7 @@ async function checkPage( pageContext: Context, pageMap: Record, redirects: Record, - options: { checkAnchors: boolean }, + options: { checkAnchors: boolean; version?: string; language?: string }, ): Promise<{ brokenLinks: BrokenLink[] redirectLinks: BrokenLink[] @@ -314,7 +314,13 @@ async function checkPage( } const normalized = normalizeLinkPath(link.href) - const result = checkInternalLink(normalized, pageMap, redirects) + const result = checkInternalLink( + normalized, + pageMap, + redirects, + options.version, + options.language, + ) if (!result.exists) { brokenLinks.push({ @@ -336,7 +342,12 @@ async function checkPage( // Direct (non-redirect) hit with a fragment: defer a cross-page anchor check. // We can't validate it now because the target page may not have been rendered // yet, so collect it and validate after the whole version finishes. - const targetKey = resolveInternalLinkKey(link.href, pageMap) + const targetKey = resolveInternalLinkKey( + link.href, + pageMap, + options.version, + options.language, + ) if (targetKey) { crossPageAnchors.push({ targetKey, @@ -432,7 +443,11 @@ async function checkVersion( // pageMap and redirects are read-only and safe to share. const pageContext = { ...baseContext, page } as Context - const result = await checkPage(page, permalink, pageContext, pageMap, redirects, options) + const result = await checkPage(page, permalink, pageContext, pageMap, redirects, { + ...options, + version, + language, + }) // Merging results here is safe: JS is single-threaded so array pushes // between await points cannot interleave with another worker's pushes. diff --git a/src/links/tests/extract-links.ts b/src/links/tests/extract-links.ts index 638aad86aae0..e79c1e4082be 100644 --- a/src/links/tests/extract-links.ts +++ b/src/links/tests/extract-links.ts @@ -504,6 +504,144 @@ describe('checkInternalLink', () => { expect(result.redirectTarget).toBe('/actions/current-path') }) + describe('version-aware resolution', () => { + // A non-FPT page has no versionless permalink, so a versionless link to it only + // resolves once you know which version is being checked. The versionless form is + // also in the redirect table as a fallback, which is what made these look like + // redirects that needed updating. + const versionedPageMap = { + '/en/enterprise-server@3.21/billing/set-up-payment': {} as unknown as Page, + '/en/actions/fpt-only': {} as unknown as Page, + } + const versionedRedirects = { + '/billing/set-up-payment': '/enterprise-cloud@latest/billing/set-up-payment', + } + + test('reports a redirect when no version is supplied (the old behavior)', () => { + const result = checkInternalLink( + '/billing/set-up-payment', + versionedPageMap, + versionedRedirects, + ) + expect(result.isRedirect).toBe(true) + }) + + test('resolves a versionless link inside the version being checked', () => { + const result = checkInternalLink( + '/billing/set-up-payment', + versionedPageMap, + versionedRedirects, + 'enterprise-server@3.21', + ) + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(false) + }) + + test('omits the version segment for FPT, matching permalink construction', () => { + const result = checkInternalLink( + '/actions/fpt-only', + versionedPageMap, + versionedRedirects, + 'free-pro-team@latest', + ) + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(false) + }) + + test('does not reinterpret a link that already names a version', () => { + const result = checkInternalLink( + '/enterprise-cloud@latest/billing/set-up-payment', + versionedPageMap, + versionedRedirects, + 'enterprise-server@3.21', + ) + expect(result.exists).toBe(false) + }) + + test('does not reinterpret a link that already names a language', () => { + const result = checkInternalLink( + '/en/actions/fpt-only', + versionedPageMap, + versionedRedirects, + 'enterprise-server@3.21', + ) + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(false) + }) + + test('still reports a genuinely broken link', () => { + const result = checkInternalLink( + '/billing/no-such-page', + versionedPageMap, + versionedRedirects, + 'enterprise-server@3.21', + ) + expect(result.exists).toBe(false) + }) + + test('still reports a genuine rename redirect', () => { + const result = checkInternalLink('/old-path', pageMap, redirects, 'free-pro-team@latest') + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(true) + expect(result.redirectTarget).toBe('/en/new-path') + }) + + test('respects a non-English language when building the key', () => { + const result = checkInternalLink( + '/billing/set-up-payment', + { '/ja/enterprise-server@3.21/billing/set-up-payment': {} as unknown as Page }, + versionedRedirects, + 'enterprise-server@3.21', + 'ja', + ) + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(false) + }) + + test('a redirect on the effective versioned URL wins over the page', () => { + // The redirect middleware runs before a page is served, so mirror that order. + const result = checkInternalLink( + '/billing/set-up-payment', + versionedPageMap, + { + '/enterprise-server@3.21/billing/set-up-payment': + '/enterprise-server@3.21/billing/renamed', + }, + 'enterprise-server@3.21', + ) + expect(result.isRedirect).toBe(true) + expect(result.redirectTarget).toBe('/enterprise-server@3.21/billing/renamed') + }) + + test('ignores a self-redirect on the effective versioned URL', () => { + const result = checkInternalLink( + '/billing/set-up-payment', + versionedPageMap, + { + '/enterprise-server@3.21/billing/set-up-payment': + '/enterprise-server@3.21/billing/set-up-payment', + }, + 'enterprise-server@3.21', + ) + expect(result.exists).toBe(true) + expect(result.isRedirect).toBe(false) + }) + + test('resolveInternalLinkKey finds the versioned key so fragments get checked', () => { + expect( + resolveInternalLinkKey( + '/billing/set-up-payment', + versionedPageMap, + 'enterprise-server@3.21', + ), + ).toBe('/en/enterprise-server@3.21/billing/set-up-payment') + }) + + test('resolveInternalLinkKey still returns null without a version', () => { + expect(resolveInternalLinkKey('/billing/set-up-payment', versionedPageMap)).toBe(null) + }) + }) + test('treats archived Enterprise Server versions as valid', () => { // Deprecated GHES versions are served by the archived enterprise versions // system, which isn't loaded into pageMap. They must not be reported broken. @@ -623,3 +761,29 @@ describe('checkAssetLink', () => { expect(checkAssetLink('/actions/getting-started')).toBe(false) }) }) + +describe('resolveInternalLinkKey version precedence', () => { + // Both keys exist because the target page applies to FPT and to GHES. + const pageMap = { + '/en/get-started/shared': {} as unknown as Page, + '/en/enterprise-server@3.21/get-started/shared': {} as unknown as Page, + } + + test('resolves to the version being checked, not the versionless key', () => { + expect(resolveInternalLinkKey('/get-started/shared', pageMap, 'enterprise-server@3.21')).toBe( + '/en/enterprise-server@3.21/get-started/shared', + ) + }) + + test('resolves to the versionless key on FPT', () => { + expect(resolveInternalLinkKey('/get-started/shared', pageMap, 'free-pro-team@latest')).toBe( + '/en/get-started/shared', + ) + }) + + test('falls back to the versionless key when the version has no page', () => { + expect(resolveInternalLinkKey('/get-started/shared', pageMap, 'enterprise-server@3.17')).toBe( + '/en/get-started/shared', + ) + }) +}) diff --git a/src/links/tests/link-report.ts b/src/links/tests/link-report.ts index 7d167fc09b61..218d13e854eb 100644 --- a/src/links/tests/link-report.ts +++ b/src/links/tests/link-report.ts @@ -160,6 +160,65 @@ describe('generateInternalLinkReport', () => { expect(report.groups).toHaveLength(0) expect(report.summary).toContain('valid') }) + + test('labels the title with version and language when supplied', () => { + // The workflow concatenates every version's report into one issue, so an + // unlabelled title leaves no way to tell the sections apart. + const report = generateInternalLinkReport([{ href: '/broken', file: 'a.md', lines: [1] }], { + version: 'enterprise-server@3.21', + language: 'en', + }) + + expect(report.title).toBe( + 'Internal Link Check (enterprise-server@3.21 en): 1 broken, 0 redirects', + ) + }) + + test('omits the label when no version or language is supplied', () => { + const report = generateInternalLinkReport([]) + + expect(report.title).toBe('Internal Link Check: 0 broken, 0 redirects') + }) +}) + +describe('createRedirectSuggestion', () => { + const linkTo = (href: string, redirectTarget: string): BrokenLink[] => [ + { href, file: 'a.md', lines: [1], isRedirect: true, redirectTarget }, + ] + + test('does not tell authors to hardcode a version', () => { + // Following "update to the new path" here bakes 3.21 into content, which breaks + // as soon as 3.22 ships. + const report = generateInternalLinkReport( + linkTo('/admin/all-releases', '/enterprise-server@3.21/admin/all-releases'), + ) + + const suggestion = report.groups[0].suggestion + expect(suggestion).toContain('Leave the link versionless') + expect(suggestion).not.toContain('Consider updating to the new path') + }) + + test('still suggests updating a genuine rename', () => { + const report = generateInternalLinkReport(linkTo('/old-name', '/new-name')) + + expect(report.groups[0].suggestion).toContain('Consider updating to the new path') + }) + + test('treats a version-only change as version resolution, not a rename', () => { + const report = generateInternalLinkReport( + linkTo('/billing/set-up', '/enterprise-cloud@latest/billing/set-up'), + ) + + expect(report.groups[0].suggestion).toContain('Leave the link versionless') + }) + + test('treats a same-version path change as a rename', () => { + const report = generateInternalLinkReport( + linkTo('/enterprise-server@3.21/old', '/enterprise-server@3.21/new'), + ) + + expect(report.groups[0].suggestion).toContain('Consider updating to the new path') + }) }) describe('generateExternalLinkReport', () => { @@ -421,3 +480,35 @@ describe('generateSampleReports', () => { expect(samples.prComment).toContain('link-checker-pr-comment') }) }) + +describe('rename advice and inherited version prefixes', () => { + const suggestionFor = (href: string, redirectTarget: string): string | undefined => + groupBrokenLinks([ + { href, file: 'admin/a.md', lines: [1], isRedirect: true, redirectTarget }, + ])[0].suggestion + + test('strips the inherited version from a versionless rename', () => { + const s = suggestionFor('/admin/old', '/enterprise-server@3.21/admin/new') + expect(s).toContain('`/admin/new`') + expect(s).not.toContain('`/enterprise-server@3.21/admin/new`') + expect(s).toContain('enterprise-server@3.21') + }) + + test('keeps the target verbatim when the link already named a version', () => { + const s = suggestionFor( + '/enterprise-server@3.21/admin/old', + '/enterprise-cloud@latest/admin/new', + ) + expect(s).toContain('`/enterprise-cloud@latest/admin/new`') + }) + + test('keeps the target verbatim when the redirect carries no version', () => { + const s = suggestionFor('/admin/old', '/admin/new') + expect(s).toContain('`/admin/new`') + }) + + test('still treats a pure version prefix as version-only', () => { + const s = suggestionFor('/admin/same', '/enterprise-server@3.21/admin/same') + expect(s).toContain('Leave the link versionless') + }) +}) diff --git a/src/links/tests/page-anchors.ts b/src/links/tests/page-anchors.ts index ccfa022ac52f..8f8d2f152752 100644 --- a/src/links/tests/page-anchors.ts +++ b/src/links/tests/page-anchors.ts @@ -149,6 +149,26 @@ describe('resolveLinkKeyForVersion', () => { expect(resolveLinkKeyForVersion('/nope/nope', 'free-pro-team@latest', pageMap)).toBe(null) }) + // A target that applies to both FPT and an enterprise version has a key for each. + // The enterprise key has to win during that version's run, otherwise the anchor is + // looked up under the FPT key while the heading cache holds the enterprise permalink. + const sharedPageMap = { + '/en/get-started/shared': {} as Page, + '/en/enterprise-server@3.17/get-started/shared': {} as Page, + } + + test('prefers the source version over the versionless key', () => { + expect( + resolveLinkKeyForVersion('/get-started/shared', 'enterprise-server@3.17', sharedPageMap), + ).toBe('/en/enterprise-server@3.17/get-started/shared') + }) + + test('still resolves to the versionless key on FPT', () => { + expect( + resolveLinkKeyForVersion('/get-started/shared', 'free-pro-team@latest', sharedPageMap), + ).toBe('/en/get-started/shared') + }) + test('ignores a fragment and query string', () => { expect( resolveLinkKeyForVersion('/admin/bar?x=1#some-heading', 'enterprise-cloud@latest', pageMap), From cd21388a3c9291f083a13fd887be2e480c45568a Mon Sep 17 00:00:00 2001 From: docs-bot <77750099+docs-bot@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:54:40 +0000 Subject: [PATCH 2/8] Update CodeQL query tables (#62820) Co-authored-by: Sarita Iyer <66540150+saritai@users.noreply.github.com> --- data/reusables/code-quality/codeql-query-tables/csharp.md | 1 - .../code-scanning/codeql-query-tables/actions.md | 8 ++++---- data/reusables/code-scanning/codeql-query-tables/cpp.md | 4 ++-- .../reusables/code-scanning/codeql-query-tables/csharp.md | 2 +- .../code-scanning/codeql-query-tables/javascript.md | 1 + 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/data/reusables/code-quality/codeql-query-tables/csharp.md b/data/reusables/code-quality/codeql-query-tables/csharp.md index 5ac878a0e897..219ed29ee969 100644 --- a/data/reusables/code-quality/codeql-query-tables/csharp.md +++ b/data/reusables/code-quality/codeql-query-tables/csharp.md @@ -25,7 +25,6 @@ | [Static field written by instance method](https://codeql.github.com/codeql-query-help/csharp/cs-static-field-written-by-instance/) | Maintainability | Recommendation | | [Unnecessarily complex Boolean expression](https://codeql.github.com/codeql-query-help/csharp/cs-simplifiable-boolean-expression/) | Maintainability | Recommendation | | [Unused label](https://codeql.github.com/codeql-query-help/csharp/cs-unused-label/) | Maintainability | Warning | -| [Useless assignment to local variable](https://codeql.github.com/codeql-query-help/csharp/cs-useless-assignment-to-local/) | Maintainability | Warning | | [Useless call to GetHashCode()](https://codeql.github.com/codeql-query-help/csharp/cs-useless-gethashcode-call/) | Maintainability | Recommendation | | [A lock is held during a wait](https://codeql.github.com/codeql-query-help/csharp/cs-locked-wait/) | Reliability | Warning | | [Call to 'System.IO.Path.Combine' may silently drop its earlier arguments](https://codeql.github.com/codeql-query-help/csharp/cs-path-combine/) | Reliability | Recommendation | diff --git a/data/reusables/code-scanning/codeql-query-tables/actions.md b/data/reusables/code-scanning/codeql-query-tables/actions.md index dd661ce729bd..c0ce35e85354 100644 --- a/data/reusables/code-scanning/codeql-query-tables/actions.md +++ b/data/reusables/code-scanning/codeql-query-tables/actions.md @@ -4,10 +4,10 @@ | --- | --- | --- | --- | --- | | [Artifact poisoning](https://codeql.github.com/codeql-query-help/actions/actions-artifact-poisoning-critical/) | 829 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Cache Poisoning via caching of untrusted files](https://codeql.github.com/codeql-query-help/actions/actions-cache-poisoning-direct-cache/) | 349 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Cache Poisoning via code injection](https://codeql.github.com/codeql-query-help/actions/actions-cache-poisoning-code-injection/) | 349, 094 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Cache Poisoning via execution of untrusted code](https://codeql.github.com/codeql-query-help/actions/actions-cache-poisoning-poisonable-step/) | 349 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Cache Poisoning via low-privileged code injection](https://codeql.github.com/codeql-query-help/actions/actions-cache-poisoning-code-injection/) | 349, 094 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Checkout of untrusted code in a privileged context](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-critical/) | 829 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | | [Checkout of untrusted code in a privileged context](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-high/) | 829 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | +| [Checkout of untrusted code in a privileged context](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-critical/) | 829 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | | [Code injection](https://codeql.github.com/codeql-query-help/actions/actions-code-injection-critical/) | 094, 095, 116 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Environment variable built from user-controlled sources](https://codeql.github.com/codeql-query-help/actions/actions-envvar-injection-critical/) | 077, 020 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Excessive Secrets Exposure](https://codeql.github.com/codeql-query-help/actions/actions-excessive-secrets-exposure/) | 312 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | @@ -15,12 +15,12 @@ | [PATH environment variable built from user-controlled sources](https://codeql.github.com/codeql-query-help/actions/actions-envpath-injection-critical/) | 077, 020 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Storage of sensitive information in GitHub Actions artifact](https://codeql.github.com/codeql-query-help/actions/actions-secrets-in-artifacts/) | 312 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Unmasked Secret Exposure](https://codeql.github.com/codeql-query-help/actions/actions-unmasked-secret-exposure/) | 312 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Untrusted Checkout TOCTOU](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-toctou-critical/) | 367 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Untrusted Checkout TOCTOU](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-toctou-high/) | 367 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Untrusted Checkout TOCTOU](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-toctou-critical/) | 367 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Use of a known vulnerable action](https://codeql.github.com/codeql-query-help/actions/actions-vulnerable-action/) | 1395 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Workflow does not contain permissions](https://codeql.github.com/codeql-query-help/actions/actions-missing-workflow-permissions/) | 275 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Artifact poisoning](https://codeql.github.com/codeql-query-help/actions/actions-artifact-poisoning-medium/) | 829 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Checkout of untrusted code in a trusted context](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-medium/) | 829 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Checkout of untrusted code in a non-privileged context](https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-medium/) | 829 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Code injection](https://codeql.github.com/codeql-query-help/actions/actions-code-injection-medium/) | 094, 095, 116 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Environment variable built from user-controlled sources](https://codeql.github.com/codeql-query-help/actions/actions-envvar-injection-medium/) | 077, 020 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [PATH environment variable built from user-controlled sources](https://codeql.github.com/codeql-query-help/actions/actions-envpath-injection-medium/) | 077, 020 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | diff --git a/data/reusables/code-scanning/codeql-query-tables/cpp.md b/data/reusables/code-scanning/codeql-query-tables/cpp.md index b2377b166a48..70af7c15f7dd 100644 --- a/data/reusables/code-scanning/codeql-query-tables/cpp.md +++ b/data/reusables/code-scanning/codeql-query-tables/cpp.md @@ -20,7 +20,7 @@ | [Incorrect return-value check for a 'scanf'-like function](https://codeql.github.com/codeql-query-help/cpp/cpp-incorrectly-checked-scanf/) | 253 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Iterator to expired container](https://codeql.github.com/codeql-query-help/cpp/cpp-iterator-to-expired-container/) | 416, 664 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Likely overrunning write](https://codeql.github.com/codeql-query-help/cpp/cpp-very-likely-overrunning-write/) | 120, 787, 805 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Mismatching new/free or malloc/delete](https://codeql.github.com/codeql-query-help/cpp/cpp-new-free-mismatch/) | 401 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Mismatching new/free or malloc/delete](https://codeql.github.com/codeql-query-help/cpp/cpp-new-free-mismatch/) | 762 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Multiplication result converted to larger type](https://codeql.github.com/codeql-query-help/cpp/cpp-integer-multiplication-cast-to-long/) | 190, 192, 197, 681 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [No space for zero terminator](https://codeql.github.com/codeql-query-help/cpp/cpp-no-space-for-terminator/) | 131, 120, 122 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Pointer overflow check](https://codeql.github.com/codeql-query-help/cpp/cpp-pointer-overflow-check/) | 758 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | @@ -85,7 +85,7 @@ | [Unbounded write](https://codeql.github.com/codeql-query-help/cpp/cpp-unbounded-write/) | 120, 787, 805 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Uncontrolled allocation size](https://codeql.github.com/codeql-query-help/cpp/cpp-uncontrolled-allocation-size/) | 190, 789 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Uncontrolled data used in path expression](https://codeql.github.com/codeql-query-help/cpp/cpp-path-injection/) | 022, 023, 036, 073 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Uncontrolled process operation](https://codeql.github.com/codeql-query-help/cpp/cpp-uncontrolled-process-operation/) | 114 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Uncontrolled process operation](https://codeql.github.com/codeql-query-help/cpp/cpp-uncontrolled-process-operation/) | 073, 078, 114 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Unterminated variadic call](https://codeql.github.com/codeql-query-help/cpp/cpp-unterminated-variadic-call/) | 121 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Untrusted input for a condition](https://codeql.github.com/codeql-query-help/cpp/cpp-tainted-permissions-check/) | 807 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Use of potentially dangerous function](https://codeql.github.com/codeql-query-help/cpp/cpp-potentially-dangerous-function/) | 676 | {% octicon "x" aria-label="Not included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | diff --git a/data/reusables/code-scanning/codeql-query-tables/csharp.md b/data/reusables/code-scanning/codeql-query-tables/csharp.md index 0e3183d00b62..df470714c0a5 100644 --- a/data/reusables/code-scanning/codeql-query-tables/csharp.md +++ b/data/reusables/code-scanning/codeql-query-tables/csharp.md @@ -5,7 +5,7 @@ | ['requireSSL' attribute is not set to true](https://codeql.github.com/codeql-query-help/csharp/cs-web-requiressl-not-set/) | 319, 614 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Arbitrary file access during archive extraction ("Zip Slip")](https://codeql.github.com/codeql-query-help/csharp/cs-zipslip/) | 022 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [ASP.NET config file enables directory browsing](https://codeql.github.com/codeql-query-help/csharp/cs-web-directory-browse-enabled/) | 548 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | -| [Assembly path injection](https://codeql.github.com/codeql-query-help/csharp/cs-assembly-path-injection/) | 114 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [Assembly path injection](https://codeql.github.com/codeql-query-help/csharp/cs-assembly-path-injection/) | 073, 114 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Clear text storage of sensitive information](https://codeql.github.com/codeql-query-help/csharp/cs-cleartext-storage-of-sensitive-information/) | 312, 315, 359 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Cookie 'HttpOnly' attribute is not set to true](https://codeql.github.com/codeql-query-help/csharp/cs-web-cookie-httponly-not-set/) | 1004 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | | [Cookie 'Secure' attribute is not set to true](https://codeql.github.com/codeql-query-help/csharp/cs-web-cookie-secure-not-set/) | 319, 614 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | diff --git a/data/reusables/code-scanning/codeql-query-tables/javascript.md b/data/reusables/code-scanning/codeql-query-tables/javascript.md index d4cb5d338ef4..35387ff52459 100644 --- a/data/reusables/code-scanning/codeql-query-tables/javascript.md +++ b/data/reusables/code-scanning/codeql-query-tables/javascript.md @@ -66,6 +66,7 @@ | [Shell command built from environment values](https://codeql.github.com/codeql-query-help/javascript/js-shell-command-injection-from-environment/) | 078, 088 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Storage of sensitive information in build artifact](https://codeql.github.com/codeql-query-help/javascript/js-build-artifact-leak/) | 312, 315, 359 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Stored cross-site scripting](https://codeql.github.com/codeql-query-help/javascript/js-stored-xss/) | 079, 116 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | +| [System prompt injection](https://codeql.github.com/codeql-query-help/javascript/js-system-prompt-injection/) | 1427 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "x" aria-label="Not included" %} | | [Template Object Injection](https://codeql.github.com/codeql-query-help/javascript/js-template-object-injection/) | 073, 094 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Type confusion through parameter tampering](https://codeql.github.com/codeql-query-help/javascript/js-type-confusion-through-parameter-tampering/) | 843 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | | [Uncontrolled command line](https://codeql.github.com/codeql-query-help/javascript/js-command-line-injection/) | 078, 088 | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | {% octicon "check" aria-label="Included" %} | From 99776aba59f75717dba82c9d09fdab72197093de Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 19:14:24 +0000 Subject: [PATCH 3/8] Group the internal link report by fix strategy (#62768) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: heiskr <1221423+heiskr@users.noreply.github.com> Copilot-Session: 026fdb7b-a47f-4af1-bf87-caa90fbbdf7f --- .gitignore | 3 + src/links/lib/extract-links.ts | 17 +- src/links/lib/link-report.ts | 247 +++++++++++++++++- src/links/scripts/check-links-internal.ts | 1 + src/links/tests/extract-links.ts | 25 ++ src/links/tests/link-report.ts | 294 +++++++++++++++++++++- 6 files changed, 574 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index ec252cdb5d5a..59d1410157ae 100644 --- a/.gitignore +++ b/.gitignore @@ -111,3 +111,6 @@ translations/ .local docs-ghes-3.14/ docs-ghes-3.15/ + +# Local output from the internal link checker +artifacts/ diff --git a/src/links/lib/extract-links.ts b/src/links/lib/extract-links.ts index 6ac63609e8ec..9b3dc275c185 100644 --- a/src/links/lib/extract-links.ts +++ b/src/links/lib/extract-links.ts @@ -535,7 +535,12 @@ export function checkInternalLink( redirects: Record, version?: string, language = 'en', -): { exists: boolean; isRedirect: boolean; redirectTarget?: string } { +): { + exists: boolean + isRedirect: boolean + redirectTarget?: string + requiresVersionContext?: boolean +} { const normalized = normalizeLinkPath(href) // Resolve enterprise-server@latest to actual version, mirroring runtime behavior. @@ -563,7 +568,15 @@ export function checkInternalLink( // self-redirect is a no-op and doesn't count. const versionedRedirect = redirects[versioned.withoutLanguage] if (versionedRedirect && versionedRedirect !== versioned.withoutLanguage) { - return { exists: true, isRedirect: true, redirectTarget: versionedRedirect } + // `update-internal-links` only ever looks the raw href up as written, so it never + // sees a redirect that exists solely under a version prefix. Say so, otherwise the + // report tells people to run a codemod that will silently leave the link alone. + return { + exists: true, + isRedirect: true, + redirectTarget: versionedRedirect, + requiresVersionContext: !(resolved in redirects), + } } if (pageMap[versioned.key]) { return { exists: true, isRedirect: false } diff --git a/src/links/lib/link-report.ts b/src/links/lib/link-report.ts index 69d6a8588eda..4ce641d0cab1 100644 --- a/src/links/lib/link-report.ts +++ b/src/links/lib/link-report.ts @@ -17,6 +17,11 @@ export interface BrokenLink { isAutotitle?: boolean isRedirect?: boolean redirectTarget?: string + /** + * The redirect was only found by resolving the href inside the version being checked. + * `update-internal-links` looks the href up exactly as written, so it can't fix these. + */ + requiresVersionContext?: boolean statusCode?: number errorMessage?: string } @@ -424,6 +429,237 @@ export function generateExternalLinkReport( } } +// ============================================================================ +// Fix strategy grouping +// ============================================================================ + +/** + * How a writer actually fixes a group. + * + * Grouping by target URL produces one section per broken URL, which is why the report runs + * to hundreds of sections that all look equally urgent. Grouping by fix strategy instead + * means each section is one decision: run a command, repoint a heading anchor, or choose a + * new destination by hand. + */ +export type FixStrategy = 'codemod' | 'versionless' | 'anchor' | 'decide' + +/** + * Past this many docsets, listing one command per docset is noisier than a single pass over + * all of `content`. + */ +const MAX_LISTED_CODEMOD_PATHS = 8 + +export function classifyFixStrategy(group: GroupedBrokenLinks): FixStrategy { + const redirectTargets = group.occurrences + .map((occ) => occ.redirectTarget) + .filter((target): target is string => Boolean(target)) + + if (group.isWarning && redirectTargets.length > 0) { + // The path is unchanged and the redirect only adds a version. Rewriting these would + // hardcode a version into content, which breaks when the next release ships. The + // codemod leaves them alone, so promising that it fixes them is a lie. + // + // Every target has to be version-only, not just the first. A group can span versions, + // and a link that merely gains a version prefix in one version but points at a renamed + // page in another is real work. Ties go to the actionable bucket. + if (redirectTargets.every((target) => isVersionOnlyRedirect(group.target, target))) { + return 'versionless' + } + // A redirect to a genuinely different path. `update-internal-links` rewrites these + // with no human judgment involved, but only when it can find the redirect from the + // href as written. If any occurrence needed version context to resolve, the codemod + // would be a no-op, so send the whole group to a human instead. + if (group.occurrences.some((occ) => occ.requiresVersionContext)) { + return 'decide' + } + return 'codemod' + } + // The link carries a fragment, so the stale part is likely a renamed heading. + if (group.target.includes('#')) { + return 'anchor' + } + return 'decide' +} + +/** + * The directories the codemod needs to be pointed at, derived from the files that actually + * contain the links. Running it against all of `content` takes minutes; running it against + * three docsets takes seconds. + * + * The checker records file paths relative to `content`, so `actions/foo.md` means + * `content/actions/foo.md`. Paths that already name a top-level directory are left alone. + */ +function codemodPaths(groups: GroupedBrokenLinks[]): string[] { + const paths = new Set() + for (const group of groups) { + for (const occ of group.occurrences) { + const segments = occ.file.split('/') + const isRooted = segments[0] === 'content' || segments[0] === 'data' + paths.add(isRooted ? segments.slice(0, 2).join('/') : `content/${segments[0]}`) + } + } + return [...paths].sort() +} + +function occurrenceCount(groups: GroupedBrokenLinks[]): number { + return groups.reduce((sum, g) => sum + g.occurrences.length, 0) +} + +function renderCodemodSection(groups: GroupedBrokenLinks[]): string { + const rows = groups + .map((group) => { + const target = group.occurrences.find((occ) => occ.redirectTarget)?.redirectTarget ?? '' + return `| \`${group.target}\` | \`${target}\` | ${group.occurrences.length} |` + }) + .join('\n') + + const flags = '--keep-stale-fragments --dont-set-autotitle' + const paths = codemodPaths(groups) + const tooManyToList = paths.length > MAX_LISTED_CODEMOD_PATHS + const commands = tooManyToList + ? `npm run update-internal-links -- content ${flags}` + : paths.map((p) => `npm run update-internal-links -- ${p} ${flags}`).join('\n') + const scopeNote = tooManyToList + ? `\nThat covers ${paths.length} docsets in one pass. To split it into reviewable pull requests, run it against one docset at a time: ${paths.map((p) => `\`${p}\``).join(', ')}.\n` + : '' + + const plural = groups.length === 1 ? '' : 's' + const occurrences = occurrenceCount(groups) + + return `## 1. Run the codemod (${groups.length} link${plural}, ${occurrences} occurrence${occurrences === 1 ? '' : 's'}) + +Every link below redirects to a known destination, so no judgment is needed. Run: + +\`\`\`bash +${commands} +\`\`\` +${scopeNote} +\`--keep-stale-fragments\` stops the codemod from silently deleting anchors it cannot verify. +That means a link like \`/old-page#heading\` becomes \`/new-page#heading\`, so if the heading +does not exist on the new page it shows up under stale anchors on the next run. +Review the diff, then open a pull request. + +
+The ${groups.length} link${plural} this fixes + +| From | To | Occurrences | +|------|-----|-------------| +${rows} + +
` +} + +/** + * Version-only redirects: the path is unchanged and the redirect just adds a version. + * + * These are not renames. A versionless link is supposed to resolve into whichever version + * the reader is on, and that is exactly what the redirect does. Rewriting them would pin + * content to a version that goes stale on the next release, so the codemod leaves them + * alone and so should writers. + */ +function renderVersionlessSection(groups: GroupedBrokenLinks[]): string { + const rows = groups + .map((group) => { + const target = group.occurrences.find((occ) => occ.redirectTarget)?.redirectTarget ?? '' + return `| \`${group.target}\` | \`${target}\` |` + }) + .join('\n') + + const plural = groups.length === 1 ? '' : 's' + const occurrences = occurrenceCount(groups) + + return `## 4. Version-only redirects (${groups.length} link${plural}, ${occurrences} occurrence${occurrences === 1 ? '' : 's'}) + +**Usually no action.** The path is unchanged: the redirect only resolves the versionless +link into the version being checked, which is what it is supposed to do. Hardcoding the +version would break when the next release ships. Change one of these only if it should +point somewhere version-specific, and use a Liquid \`ifversion\` gate when the target +should differ per version. + +
+The ${groups.length} link${plural} in this state + +| Link | Resolves to | +|------|-------------| +${rows} + +
` +} + +function renderManualSection( + heading: string, + blurb: string, + groups: GroupedBrokenLinks[], + isExternal: boolean, +): string { + const sections = groups.map((group) => TEMPLATES.group(group, isExternal)).join('\n\n') + return `## ${heading} (${groups.length} link${groups.length === 1 ? '' : 's'}, ${occurrenceCount(groups)} occurrence${occurrenceCount(groups) === 1 ? '' : 's'}) + +${blurb} + +${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 { + const codemod = groups.filter((g) => classifyFixStrategy(g) === 'codemod') + const versionless = groups.filter((g) => classifyFixStrategy(g) === 'versionless') + const anchors = groups.filter((g) => classifyFixStrategy(g) === 'anchor') + const decide = groups.filter((g) => classifyFixStrategy(g) === 'decide') + + const summaryRows = [ + codemod.length > 0 && + `| 1. Run the codemod | ${codemod.length} | ${occurrenceCount(codemod)} | Mechanical. Run the command. |`, + anchors.length > 0 && + `| 2. Fix stale anchors | ${anchors.length} | ${occurrenceCount(anchors)} | A heading was renamed. Repoint it. |`, + decide.length > 0 && + `| 3. Pick a destination | ${decide.length} | ${occurrenceCount(decide)} | The codemod cannot resolve these. Needs a human. |`, + versionless.length > 0 && + `| 4. Usually nothing | ${versionless.length} | ${occurrenceCount(versionless)} | Version-only redirects. Leave them versionless. |`, + ].filter(Boolean) as string[] + + const parts = [ + `## Start here + +| Bucket | Links | Occurrences | Effort | +|--------|-------|-------------|--------| +${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 (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, + ), + ) + } + if (decide.length > 0) { + parts.push( + renderManualSection( + '3. Links the codemod cannot fix', + '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, + ), + ) + } + + if (versionless.length > 0) { + parts.push(renderVersionlessSection(versionless)) + } + + return parts.join('\n\n') +} + // ============================================================================ // Markdown Rendering // ============================================================================ @@ -477,15 +713,20 @@ export function reportToMarkdown(report: LinkReport, isExternal = false): string return parts.join('\n') } - // Table of contents for large reports - if (report.groups.length > 5) { + // Table of contents for large reports. The internal report is grouped by fix strategy + // instead, where the three bucket headings are the navigation. + if (isExternal && report.groups.length > 5) { parts.push(TEMPLATES.tableOfContents(report.groups)) parts.push('') } // Groups if (hasBrokenOrRedirectGroups) { - parts.push(renderGroups(report.groups, isExternal)) + parts.push( + isExternal + ? renderGroups(report.groups, isExternal) + : renderByFixStrategy(report.groups, isExternal), + ) } // Self-referential links section (external report only) diff --git a/src/links/scripts/check-links-internal.ts b/src/links/scripts/check-links-internal.ts index 9e0b234827d4..c298d98066ff 100644 --- a/src/links/scripts/check-links-internal.ts +++ b/src/links/scripts/check-links-internal.ts @@ -337,6 +337,7 @@ async function checkPage( text: link.text, isRedirect: true, redirectTarget: result.redirectTarget, + requiresVersionContext: result.requiresVersionContext, }) } else if (options.checkAnchors && link.fragment) { // Direct (non-redirect) hit with a fragment: defer a cross-page anchor check. diff --git a/src/links/tests/extract-links.ts b/src/links/tests/extract-links.ts index e79c1e4082be..8227e92df8c3 100644 --- a/src/links/tests/extract-links.ts +++ b/src/links/tests/extract-links.ts @@ -762,6 +762,31 @@ describe('checkAssetLink', () => { }) }) +describe('checkInternalLink version-only redirects', () => { + const pageMap = { + '/en/enterprise-server@3.21/admin/other': {} as unknown as Page, + } + + test('flags a redirect that only exists under the version prefix', () => { + const redirects = { + '/enterprise-server@3.21/admin/old': '/enterprise-server@3.21/admin/other', + } + const result = checkInternalLink('/admin/old', pageMap, redirects, 'enterprise-server@3.21') + expect(result.isRedirect).toBe(true) + expect(result.requiresVersionContext).toBe(true) + }) + + test('does not flag it when the versionless form redirects too', () => { + const redirects = { + '/enterprise-server@3.21/admin/old': '/enterprise-server@3.21/admin/other', + '/admin/old': '/admin/other', + } + const result = checkInternalLink('/admin/old', pageMap, redirects, 'enterprise-server@3.21') + expect(result.isRedirect).toBe(true) + expect(result.requiresVersionContext).toBe(false) + }) +}) + describe('resolveInternalLinkKey version precedence', () => { // Both keys exist because the target page applies to FPT and to GHES. const pageMap = { diff --git a/src/links/tests/link-report.ts b/src/links/tests/link-report.ts index 218d13e854eb..5df737dc6db1 100644 --- a/src/links/tests/link-report.ts +++ b/src/links/tests/link-report.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'vitest' import { type BrokenLink, + type GroupedBrokenLinks, groupBrokenLinks, groupExternalLinksByDomain, generateInternalLinkReport, @@ -8,6 +9,7 @@ import { reportToMarkdown, generatePRComment, generateSampleReports, + classifyFixStrategy, } from '../lib/link-report' describe('groupBrokenLinks', () => { @@ -265,7 +267,20 @@ describe('reportToMarkdown', () => { expect(markdown).toContain('actions/runs/123') }) - test('includes table of contents for large reports', () => { + test('includes table of contents for large external reports', () => { + const links: BrokenLink[] = Array.from({ length: 10 }, (_, i) => ({ + href: `https://example${i}.com/path`, + file: `content/${i}.md`, + lines: [i], + })) + + const report = generateExternalLinkReport(links) + const markdown = reportToMarkdown(report, true) + + expect(markdown).toContain('Quick Navigation') + }) + + test('internal reports navigate by fix strategy instead of a per-link table of contents', () => { const links: BrokenLink[] = Array.from({ length: 10 }, (_, i) => ({ href: `/path/${i}`, file: `content/${i}.md`, @@ -275,7 +290,8 @@ describe('reportToMarkdown', () => { const report = generateInternalLinkReport(links) const markdown = reportToMarkdown(report) - expect(markdown).toContain('Quick Navigation') + expect(markdown).not.toContain('Quick Navigation') + expect(markdown).toContain('## Start here') }) test('formats groups with file tables', () => { @@ -298,17 +314,17 @@ describe('reportToMarkdown', () => { expect(markdown).toContain('No issues found') }) - test('separates errors and warnings into sections', () => { + test('external reports still render a broken links section', () => { const links: BrokenLink[] = [ - { href: '/broken', file: 'a.md', lines: [1] }, - { href: '/redirect', file: 'b.md', lines: [2], isRedirect: true }, + { href: 'https://broken.example/page', file: 'a.md', lines: [1] }, + { href: 'https://other.example/page', file: 'b.md', lines: [2] }, ] - const report = generateInternalLinkReport(links) - const markdown = reportToMarkdown(report) + const report = generateExternalLinkReport(links) + const markdown = reportToMarkdown(report, true) expect(markdown).toContain('## ❌ Broken Links') - expect(markdown).toContain('## ⚠️ Redirects to Update') + expect(markdown).toContain('broken.example') }) test('includes potential internal links section with no broken links', () => { @@ -481,6 +497,268 @@ describe('generateSampleReports', () => { }) }) +describe('classifyFixStrategy', () => { + const group = (over: Partial[0]>) => + ({ target: '/x', occurrences: [], isWarning: false, ...over }) as Parameters< + typeof classifyFixStrategy + >[0] + + test('a redirect with a known destination is codemod work', () => { + expect( + classifyFixStrategy( + group({ + isWarning: true, + occurrences: [ + { href: '/old', file: 'a.md', lines: [1], isRedirect: true, redirectTarget: '/new' }, + ], + }), + ), + ).toBe('codemod') + }) + + test('a redirect with no resolved destination is not codemod work', () => { + expect( + classifyFixStrategy( + group({ isWarning: true, occurrences: [{ href: '/old', file: 'a.md', lines: [1] }] }), + ), + ).toBe('decide') + }) + + test('a link carrying a fragment is an anchor problem', () => { + expect( + classifyFixStrategy( + group({ + target: '/page#gone', + occurrences: [{ href: '/page#gone', file: 'a.md', lines: [1] }], + }), + ), + ).toBe('anchor') + }) + + test('a plain broken link needs a human', () => { + expect( + classifyFixStrategy(group({ occurrences: [{ href: '/x', file: 'a.md', lines: [1] }] })), + ).toBe('decide') + }) +}) + +describe('internal report grouped by fix strategy', () => { + const links: BrokenLink[] = [ + { + href: '/old-actions', + file: 'actions/foo.md', + lines: [1], + isRedirect: true, + redirectTarget: '/new-actions', + }, + { + href: '/old-admin', + file: 'admin/bar.md', + lines: [2], + isRedirect: true, + redirectTarget: '/new-admin', + }, + { href: '/page#renamed', file: 'actions/baz.md', lines: [3] }, + { href: '/nowhere', file: 'issues/qux.md', lines: [4] }, + ] + + const markdown = reportToMarkdown(generateInternalLinkReport(links)) + + test('leads with a summary of the buckets', () => { + expect(markdown).toContain('## Start here') + expect(markdown).toContain('1. Run the codemod') + expect(markdown).toContain('2. Fix stale anchors') + expect(markdown).toContain('3. Pick a destination') + }) + + test('gives a runnable command scoped to the affected docsets', () => { + expect(markdown).toContain( + 'npm run update-internal-links -- content/actions --keep-stale-fragments --dont-set-autotitle', + ) + expect(markdown).toContain( + 'npm run update-internal-links -- content/admin --keep-stale-fragments --dont-set-autotitle', + ) + // content/issues only appears in the manual bucket, so it is not a codemod target. + expect(markdown).not.toContain('npm run update-internal-links -- content/issues ') + }) + + test('collapses codemod work into one table instead of a section per link', () => { + expect(markdown).toContain('| `/old-actions` | `/new-actions` | 1 |') + expect(markdown).not.toContain('### ⚠️ `/old-actions`') + }) + + test('keeps per-file detail for the links a human has to judge', () => { + expect(markdown).toContain('### ❌ `/nowhere`') + expect(markdown).toContain('`issues/qux.md`') + }) +}) + +describe('codemod command scoping', () => { + const redirectLink = (docset: string): BrokenLink => ({ + href: `/old-${docset}`, + file: `${docset}/page.md`, + lines: [1], + isRedirect: true, + redirectTarget: `/new-${docset}`, + }) + + test('paths are rooted at content, since the checker reports content-relative files', () => { + const markdown = reportToMarkdown(generateInternalLinkReport([redirectLink('actions')])) + + expect(markdown).toContain('npm run update-internal-links -- content/actions ') + expect(markdown).not.toContain('npm run update-internal-links -- actions ') + }) + + test('falls back to a single pass when too many docsets are affected', () => { + const docsets = Array.from({ length: 12 }, (_, i) => `docset-${i}`) + const markdown = reportToMarkdown(generateInternalLinkReport(docsets.map(redirectLink))) + + expect(markdown).toContain( + 'npm run update-internal-links -- content --keep-stale-fragments --dont-set-autotitle', + ) + expect(markdown).toContain('That covers 12 docsets in one pass.') + expect(markdown).toContain('`content/docset-0`') + }) +}) + +describe('version-only redirects', () => { + const versionOnly: BrokenLink[] = [ + { + href: '/admin/overview', + file: 'actions/a.md', + lines: [1], + isRedirect: true, + redirectTarget: '/enterprise-cloud@latest/admin/overview', + }, + ] + + const renamed: BrokenLink[] = [ + { + href: '/old-path', + file: 'actions/b.md', + lines: [2], + isRedirect: true, + redirectTarget: '/new-path', + }, + ] + + test('a redirect that only adds a version is not codemod work', () => { + const [group] = generateInternalLinkReport(versionOnly).groups + expect(classifyFixStrategy(group)).toBe('versionless') + }) + + test('a redirect to a different path is still codemod work', () => { + const [group] = generateInternalLinkReport(renamed).groups + expect(classifyFixStrategy(group)).toBe('codemod') + }) + + test('version-only links are excluded from the codemod count and command', () => { + const markdown = reportToMarkdown(generateInternalLinkReport([...versionOnly, ...renamed])) + + expect(markdown).toContain('## 1. Run the codemod (1 link, 1 occurrence)') + expect(markdown).toContain('## 4. Version-only redirects (1 link, 1 occurrence)') + // The codemod does not touch these, so it must not claim to fix them. + expect(markdown).not.toContain( + '| `/admin/overview` | `/enterprise-cloud@latest/admin/overview` | 1 |', + ) + }) + + test('tells writers to leave them versionless rather than hardcode a version', () => { + const markdown = reportToMarkdown(generateInternalLinkReport(versionOnly)) + + expect(markdown).toContain('Usually no action.') + expect(markdown).toContain('ifversion') + expect(markdown).not.toContain('## 1. Run the codemod') + }) +}) + +describe('version-only classification across versions', () => { + test('a link that is version-only in one version and renamed in another is codemod work', () => { + // Merged reports put every version's occurrences in one group. Classifying on the first + // redirect target alone would file this under "no action" and hide the rename. + const mixed: BrokenLink[] = [ + { + href: '/admin/overview', + file: 'actions/a.md', + lines: [1], + isRedirect: true, + redirectTarget: '/enterprise-cloud@latest/admin/overview', + }, + { + href: '/admin/overview', + file: 'actions/b.md', + lines: [1], + isRedirect: true, + redirectTarget: '/admin/renamed-overview', + }, + ] + + const [group] = generateInternalLinkReport(mixed).groups + expect(group.occurrences).toHaveLength(2) + expect(classifyFixStrategy(group)).toBe('codemod') + }) + + test('differing version prefixes for the same path stay versionless', () => { + const perVersion: BrokenLink[] = [ + { + href: '/admin/overview', + file: 'actions/a.md', + lines: [1], + isRedirect: true, + redirectTarget: '/enterprise-cloud@latest/admin/overview', + }, + { + href: '/admin/overview', + file: 'actions/b.md', + lines: [1], + isRedirect: true, + redirectTarget: '/enterprise-server@3.22/admin/overview', + }, + ] + + const [group] = generateInternalLinkReport(perVersion).groups + expect(classifyFixStrategy(group)).toBe('versionless') + }) +}) + +describe('redirects the codemod cannot resolve', () => { + const occurrence = (extra: Partial = {}): BrokenLink => ({ + href: '/admin/old', + file: 'admin/foo.md', + lines: [3], + isRedirect: true, + redirectTarget: '/enterprise-server@3.21/admin/other', + ...extra, + }) + + test('classifies a version-only redirect as manual work, not codemod work', () => { + const group: GroupedBrokenLinks = { + target: '/admin/old', + occurrences: [occurrence({ requiresVersionContext: true })], + isWarning: true, + } + expect(classifyFixStrategy(group)).toBe('decide') + }) + + test('still classifies a plain rename redirect as codemod work', () => { + const group: GroupedBrokenLinks = { + target: '/admin/old', + occurrences: [occurrence()], + isWarning: true, + } + expect(classifyFixStrategy(group)).toBe('codemod') + }) + + test('one unresolvable occurrence sends the whole merged group to a human', () => { + const group: GroupedBrokenLinks = { + target: '/admin/old', + occurrences: [occurrence(), occurrence({ requiresVersionContext: true })], + isWarning: true, + } + expect(classifyFixStrategy(group)).toBe('decide') + }) +}) + describe('rename advice and inherited version prefixes', () => { const suggestionFor = (href: string, redirectTarget: string): string | undefined => groupBrokenLinks([ From 7a5fd3444d0643dc3b7d141a796bf5b63ba427fd Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 19:14:27 +0000 Subject: [PATCH 4/8] Fix reordered Liquid tags in pt and ko intros (SCRAPE-6781) (#62703) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3965efd1-46d4-4ea9-b2c1-60af4b91e032 --- .../lib/correct-translation-content.ts | 27 ++++++++++++++++ .../tests/correct-translation-content.ts | 31 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/languages/lib/correct-translation-content.ts b/src/languages/lib/correct-translation-content.ts index f6d2aa5510a7..93cb80520dc4 100644 --- a/src/languages/lib/correct-translation-content.ts +++ b/src/languages/lib/correct-translation-content.ts @@ -773,6 +773,19 @@ export function correctTranslatedContentStrings( 'tornando mais difícil para os atores mal-intencionados acessarem os repositórios e as configurações de uma organização.', 'tornando mais difícil para os atores mal-intencionados acessarem os repositórios e as configurações de uma organização.{% endif %}', ) + + // [SCRAPE-6781] Per-file fix: + // codespaces/managing-codespaces-for-your-organization/enabling-or-disabling-github-codespaces-for-your-organization.md + // (intro): the translator reordered the inline Liquid tags to match + // Portuguese word order, so `{% endif %}` lands before the + // `{% ifversion ghec %}` that opens the block. English source is + // `...private {% ifversion ghec %}and internal {% endif %}repositories`. + // Reorder the tags around the existing translated words so ghec reads + // "privados e internos" and fpt reads "privados". + content = content.replaceAll( + 'nos repositórios internos e {% endif %}privados {% ifversion ghec %}da sua organização.', + 'nos repositórios privados {% ifversion ghec %}e internos {% endif %}da sua organização.', + ) } if (context.code === 'zh') { @@ -1735,6 +1748,20 @@ export function correctTranslatedContentStrings( ) { content = content.replace(/\{%-?\s*endif\s*-?%\}\s*(\{%-?\s*ifversion\s)/g, '$1') } + + // [SCRAPE-6781] Per-file fix: + // organizations/managing-membership-in-your-organization/reinstating-a-former-member-of-your-organization.md + // (intro): the translator reordered the inline Liquid tags to match Korean + // word order, so `{% else %}` and `{% endif%}` both land before the + // `{% ifversion fpt or ghec %}` that opens the block, which broke the + // /ko/organizations landing page scrape. Move the opener to the front so + // fpt/ghec reads "이전 조직 구성원을 초대하여 다시 추가하고" and ghes reads + // "조직에 이전 멤버를 다시 추가하고". Note the English source also writes + // `{% endif%}` without a leading space, which the translation preserved. + content = content.replaceAll( + '이전 조직 구성원을 초대하여{% else %}조직에 이전 멤버를{% endif%} 다시 추가하고 해당 사용자의 이전 역할, 액세스 권한, 포크 및 설정을 복원할지 여부를 선택할 수 {% ifversion fpt or ghec %}있습니다.', + '{% ifversion fpt or ghec %}이전 조직 구성원을 초대하여{% else %}조직에 이전 멤버를{% endif %} 다시 추가하고 해당 사용자의 이전 역할, 액세스 권한, 포크 및 설정을 복원할지 여부를 선택할 수 있습니다.', + ) } if (context.code === 'de') { diff --git a/src/languages/tests/correct-translation-content.ts b/src/languages/tests/correct-translation-content.ts index 0dabe73ec487..df683972fbb5 100644 --- a/src/languages/tests/correct-translation-content.ts +++ b/src/languages/tests/correct-translation-content.ts @@ -2511,6 +2511,37 @@ Para más información, consulta "[AUTOTITLE](/path)". }) }) + // ─── SCRAPE-6781: search-scrape failures ───────────────────────────── + // The pt codespaces and ko organizations landing pages failed to scrape + // (github/docs-engineering#6781). Neither landing page is itself corrupt: + // `discovery-landing` pages render their descendants' intros via + // getAllTocItems, so a corrupt child intro takes the whole landing page + // down. In both cases the translator reordered the inline Liquid tags to + // match target-language word order, leaving `{% else %}`/`{% endif %}` + // ahead of the `{% ifversion %}` that opens the block. The corrector runs + // on the PARSED intro value. + describe('SCRAPE-6781 per-file fixes', () => { + test('pt: enabling-or-disabling-github-codespaces-for-your-organization intro reorders tags', () => { + const broken = + 'Você pode controlar quais usuários podem usar {% data variables.product.prodname_github_codespaces %} nos repositórios internos e {% endif %}privados {% ifversion ghec %}da sua organização.' + const fixed = + 'Você pode controlar quais usuários podem usar {% data variables.product.prodname_github_codespaces %} nos repositórios privados {% ifversion ghec %}e internos {% endif %}da sua organização.' + expect(fix(broken, 'pt')).toBe(fixed) + // idempotent: the fix only matches the broken form + expect(fix(fixed, 'pt')).toBe(fixed) + }) + + test('ko: reinstating-a-former-member-of-your-organization intro reorders tags', () => { + const broken = + '이전 조직 구성원을 초대하여{% else %}조직에 이전 멤버를{% endif%} 다시 추가하고 해당 사용자의 이전 역할, 액세스 권한, 포크 및 설정을 복원할지 여부를 선택할 수 {% ifversion fpt or ghec %}있습니다.' + const fixed = + '{% ifversion fpt or ghec %}이전 조직 구성원을 초대하여{% else %}조직에 이전 멤버를{% endif %} 다시 추가하고 해당 사용자의 이전 역할, 액세스 권한, 포크 및 설정을 복원할지 여부를 선택할 수 있습니다.' + expect(fix(broken, 'ko')).toBe(fixed) + // idempotent: the fix only matches the broken form + expect(fix(fixed, 'ko')).toBe(fixed) + }) + }) + // ─── New patterns ─────────────────────────────────────────────────── describe('es: you-can-fork.md per-file fix', () => { From 5de5a3bf0b9856fd6d3003d02ae06ff1b664efa8 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 19:14:31 +0000 Subject: [PATCH 5/8] Remove issue analysis agent directions from scraping failure issue (#62704) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3965efd1-46d4-4ea9-b2c1-60af4b91e032 --- .github/workflows/index-general-search.yml | 31 ---------------------- 1 file changed, 31 deletions(-) diff --git a/.github/workflows/index-general-search.yml b/.github/workflows/index-general-search.yml index e3debd22bba9..e6318728ca5a 100644 --- a/.github/workflows/index-general-search.yml +++ b/.github/workflows/index-general-search.yml @@ -341,37 +341,6 @@ jobs: This issue was automatically created by the \`$WORKFLOW_NAME\` workflow. Subsequent failures from later workflow runs will be added as comments on this issue rather than opening a new issue each day. - - --- - - ## Instructions for the issue analysis agent - - These scraping failures are **almost never timeouts**. They are - almost always caused by Liquid/Markdown corruptions in the - translated source that make the page fail to render. Please do not - file this as a transient/timeout issue. - - To investigate: - - 1. Pull the translation repos locally: - \`\`\` - npm run clone-translations - \`\`\` - 2. Identify the specific corruptions with the canonical reporter - (verify the exact script name in \`package.json\` before running): - \`\`\` - npm run count-translation-corruptions - \`\`\` - The script writes a structured JSON report to stdout listing each - corrupted file, its location, the error message, and the - offending/illegal tag token when available. - 3. **Open a pull request that fixes the corruptions** rather than - just commenting the findings on this issue. The issue body above - (and any subsequent comments) already lists the affected pages; - the value the agent adds is the fix. Where possible, fix the - underlying pattern in - \`src/languages/lib/correct-translation-content.ts\` so the same - corruption is corrected on future syncs. EOF ) gh issue create \ From 1bad1b42c9bc7d5c256f0d5407c7ffe4b3c1cb80 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 19:30:44 +0000 Subject: [PATCH 6/8] Bump nanoid to 3.3.18 to fix GHSA-2v37-7h3g-55p8 (#62808) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 645c73a6-af94-4c7e-9444-66c92aabd7a9 --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index effa863b2957..fa22fe76aa2d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12275,9 +12275,9 @@ "optional": true }, "node_modules/nanoid": { - "version": "3.3.17", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.17.tgz", - "integrity": "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "funding": [ { "type": "github", From 7c1a67c8d50868edbb44b7b1787febd7f0da97c6 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Wed, 19 Aug 2026 19:32:27 +0000 Subject: [PATCH 7/8] Actually write the link fixes the codemod computes for YAML files (#62759) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 026fdb7b-a47f-4af1-bf87-caa90fbbdf7f Copilot-Session: a12f7880-548d-4f87-a578-21fd6ad0f3da --- src/links/lib/update-internal-links.ts | 145 +++++++++++++- src/links/scripts/update-internal-links.ts | 28 ++- src/links/tests/update-internal-links-yaml.ts | 184 ++++++++++++++++++ 3 files changed, 340 insertions(+), 17 deletions(-) create mode 100644 src/links/tests/update-internal-links-yaml.ts diff --git a/src/links/lib/update-internal-links.ts b/src/links/lib/update-internal-links.ts index 216e1dfeb468..f1a952ad6998 100644 --- a/src/links/lib/update-internal-links.ts +++ b/src/links/lib/update-internal-links.ts @@ -3,6 +3,7 @@ import fs from 'fs' import { visit, Test } from 'unist-util-visit' import { fromMarkdown } from 'mdast-util-from-markdown' import { toMarkdown } from 'mdast-util-to-markdown' +import { dump } from 'js-yaml' import { loadYaml } from '@/frame/lib/load-yaml' import { type Node, type Nodes, type Definition, type Link } from 'mdast' @@ -29,7 +30,7 @@ const logger = createLogger(import.meta.url) // we, at runtime, render out the links const AUTOTITLE = 'AUTOTITLE' -type LinkContext = { +export type LinkContext = { pages: Record redirects: NonNullable currentLanguage: string @@ -74,6 +75,12 @@ type PendingReplacement = { baseHref: string makeMarkdown: (href: string) => string fragment?: CarriedFragment + /** + * Byte range of this link in the source, when the node's position could be mapped back + * and the slice matches `asMarkdown` exactly. Replacing by range instead of by string + * search keeps identical text elsewhere in the file untouched. + */ + span?: [number, number] } const Options = { @@ -120,7 +127,11 @@ export async function updateInternalLinks(files: string[], options = {}) { return results } -async function updateFile(file: string, context: LinkContext, opts: typeof Options) { +/** + * Exported so tests can drive a single file with a hand-built context. Loading the real + * page tree takes tens of seconds, which is too slow to cover the rewrite branches. + */ +export async function updateFile(file: string, context: LinkContext, opts: typeof Options) { const rawContent = fs.readFileSync(file, 'utf8') let { data, content } = frontmatter(rawContent) data = data || {} @@ -132,14 +143,58 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio // the `frontmatter(rawContent).data` always becomes `{}`. // And since the Yaml file might contain arrays of internal linked // pathnames, we have to re-read it fully. - if (file.endsWith('.yml')) { + const isYaml = file.endsWith('.yml') + if (isYaml) { Object.assign(data, loadYaml(content)) } let newContent = content - const ast = fromMarkdown(newContent) + + // Captured so the closure below sees a non-reassignable string. + const source = content + + // A YAML file is parsed as Markdown to find its links, and that parse is + // indentation-sensitive: a value indented four or more spaces reads as a code block, + // so the AST holds fewer link nodes than the text has occurrences. Stripping the + // leading whitespace from every line exposes all of them. Line numbers are unaffected, + // and `sourceSpan` maps each node's columns back onto the original text so the + // rewrite still lands on the real bytes. + const parseSource = isYaml ? dedentLines(source) : source + const lineStarts = buildLineStarts(source) + const indents = isYaml ? source.split('\n').map((line) => /^[ \t]*/.exec(line)![0].length) : null + + /** + * Column of a node in the original text. The YAML parse runs on dedented lines, so the + * indent has to go back on before the column is reported to a human. + */ + function sourceColumn(node: Nodes): number | undefined { + const pos = node.position + if (!pos?.start.column) return undefined + const indent = indents ? (indents[pos.start.line - 1] ?? 0) : 0 + return pos.start.column + indent + } + + /** + * Byte range of a node in the source, or undefined when the range can't be trusted: + * a node spanning several lines, or a serialization that doesn't match the source. + */ + function sourceSpan(node: Nodes, asMarkdown: string): [number, number] | undefined { + const pos = node.position + if (!pos?.start.line || !pos.end.line || pos.start.line !== pos.end.line) return undefined + const lineIndex = pos.start.line - 1 + const lineStart = lineStarts[lineIndex] + if (lineStart === undefined) return undefined + const indent = indents ? indents[lineIndex] : 0 + const start = lineStart + indent + (pos.start.column - 1) + const end = lineStart + indent + (pos.end.column - 1) + return source.slice(start, end) === asMarkdown ? [start, end] : undefined + } + + const ast = fromMarkdown(parseSource) const replacements: Replacement[] = [] + const spanEdits: { start: number; end: number; text: string }[] = [] + const stringEdits: { find: string; text: string }[] = [] const warnings: Warning[] = [] const newData = structuredClone(data) @@ -213,7 +268,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio // getNewHref() might return a deliberate `undefined` if the // new href value could not be computed for some reason. const baseHref = result === undefined ? node.url : result.href - const column = node.position?.start.column + const column = sourceColumn(node) const line = (node.position?.start.line ?? 0) + lineOffset pending.push({ asMarkdown, @@ -222,6 +277,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio baseHref, makeMarkdown: (href) => `[${label}]: ${href}`, fragment: result?.fragment, + span: sourceSpan(node, asMarkdown), }) } }) @@ -281,7 +337,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio */ if (xValue) { if (singleStartingQuote(xValue)) { - const column = node.position?.start.column + const column = sourceColumn(node) const line = (node.position?.start.line ?? 0) + lineOffset warnings.push({ warning: 'Starts with a single " inside the text', @@ -290,7 +346,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio column, }) } else if (isSimpleQuote(xValue)) { - const column = node.position?.start.column + const column = sourceColumn(node) const line = (node.position?.start.line ?? 0) + lineOffset warnings.push({ warning: 'Starts and ends with a " inside the text', @@ -311,7 +367,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio fragment = result.fragment } } - const column = node.position?.start.column + const column = sourceColumn(node) const line = (node.position?.start.line ?? 0) + lineOffset pending.push({ asMarkdown, @@ -320,6 +376,7 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio baseHref, makeMarkdown: (href) => `[${newTitle}](${href})`, fragment, + span: sourceSpan(node, asMarkdown), }) } else if (opts.verbose) { logger.warn('Unable to find link as Markdown in the source content', { @@ -376,14 +433,33 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio } } const newAsMarkdown = item.makeMarkdown(finalHref) - if (item.asMarkdown !== newAsMarkdown) { + if (item.asMarkdown !== newAsMarkdown && content.includes(item.asMarkdown)) { replacements.push({ asMarkdown: item.asMarkdown, newAsMarkdown, line: item.line, column: item.column, }) - newContent = newContent.replace(item.asMarkdown, newAsMarkdown) + if (item.span) { + spanEdits.push({ start: item.span[0], end: item.span[1], text: newAsMarkdown }) + } else { + // No trustworthy range for this node, so fall back to a string search. Left for + // the second pass, after the ranged edits, since a search can't be offset-aware. + stringEdits.push({ find: item.asMarkdown, text: newAsMarkdown }) + } + } + } + + // Ranged edits go in descending order so earlier offsets stay valid, and each one + // touches exactly the bytes the parser identified as a link. That's what keeps an + // identical string in a comment or a code example from being rewritten too. + spanEdits.sort((a, b) => b.start - a.start) + for (const edit of spanEdits) { + newContent = newContent.slice(0, edit.start) + edit.text + newContent.slice(edit.end) + } + for (const edit of stringEdits) { + if (newContent.includes(edit.find)) { + newContent = newContent.replace(edit.find, edit.text) } } @@ -398,6 +474,23 @@ async function updateFile(file: string, context: LinkContext, opts: typeof Optio } } +/** Strip leading whitespace from every line, preserving the line count. */ +function dedentLines(content: string): string { + return content + .split('\n') + .map((line) => line.replace(/^[ \t]+/, '')) + .join('\n') +} + +/** Byte offset where each line begins, so a line/column pair can become an offset. */ +function buildLineStarts(content: string): number[] { + const starts = [0] + for (let i = 0; i < content.length; i++) { + if (content[i] === '\n') starts.push(i + 1) + } + return starts +} + function isDefinition(node: Node): node is Definition { return node.type === 'definition' } @@ -701,6 +794,38 @@ function singleStartingQuote(text: string) { function isSimpleQuote(text: string) { return text.startsWith('"') && text.endsWith('"') && text.split('"').length === 3 } + +/** + * Write a YAML data file back out. + * + * For `.yml` files every link fix lands in `newContent`, the file's own text, because + * `updateFile` finds Markdown links by parsing that text and rewrites them in place. + * `newData` is only mutated for the structured link keys (`featuredLinks` and + * `introLinks`), which no file under `data/` currently uses. + * + * Writing `dump(newData)` therefore threw away every fix and reserialized the untouched + * data instead: pure churn, no change. Prefer the surgically edited text, and only fall + * back to reserializing when the structured data genuinely changed. + */ +export function serializeYaml( + newContent: string, + newData: Record | undefined, + differentContent: boolean, + differentData: boolean, +): string { + if (!differentData) return newContent + if (differentContent) { + // The two kinds of change live in different representations and there is no + // format-preserving way to merge them, so `dump` would silently drop the text + // fixes. No file hits this today. Fail loudly rather than lose edits quietly. + throw new Error( + 'Cannot serialize a YAML file that has both text and structured data changes ' + + 'without losing one of them. This needs a format-preserving merge.', + ) + } + return dump(newData || {}) +} + /** * Write a Markdown page back out, preserving the original frontmatter text verbatim * whenever the frontmatter data itself didn't change. diff --git a/src/links/scripts/update-internal-links.ts b/src/links/scripts/update-internal-links.ts index 2b923a9bd1bf..2531b7852f82 100755 --- a/src/links/scripts/update-internal-links.ts +++ b/src/links/scripts/update-internal-links.ts @@ -12,9 +12,12 @@ import path from 'path' import { program } from 'commander' import chalk from 'chalk' -import { dump } from 'js-yaml' -import { updateInternalLinks, serializeMarkdown } from '@/links/lib/update-internal-links' +import { + updateInternalLinks, + serializeMarkdown, + serializeYaml, +} from '@/links/lib/update-internal-links' import walkFiles from '@/workflows/walk-files' program @@ -116,6 +119,10 @@ async function main(files: string[], opts: Options) { const results = await updateInternalLinks(actualFiles, options) let exitCheck = 0 + // Serializing can throw, and a throw halfway through the loop would leave a + // half-updated checkout. Every output is computed first so a failure on the last + // file means nothing was written at all, which is what the comment above promises. + const pendingWrites: { file: string; output: string }[] = [] for (const { file, rawContent, @@ -153,15 +160,17 @@ async function main(files: string[], opts: Options) { } if (!opts.dryRun) { if (file.endsWith('.yml')) { - fs.writeFileSync(file, dump(newData), 'utf-8') + pendingWrites.push({ + file, + output: serializeYaml(newContent, newData, differentContent, differentData), + }) } else { // Remember the `content` and `newContent` is the "meat" of the // Markdown page. To save it you need the frontmatter data too. - fs.writeFileSync( + pendingWrites.push({ file, - serializeMarkdown(rawContent, content, newContent, newData, differentData), - 'utf-8', - ) + output: serializeMarkdown(rawContent, content, newContent, newData, differentData), + }) } } } @@ -175,6 +184,11 @@ async function main(files: string[], opts: Options) { } } + // Every serializer succeeded, so the writes can't be interrupted by one of them. + for (const { file, output } of pendingWrites) { + fs.writeFileSync(file, output, 'utf-8') + } + if (opts.aggregateStats) { const countFiles = results.length const countChangedFiles = new Set(results.filter((result) => result.replacements.length > 0)) diff --git a/src/links/tests/update-internal-links-yaml.ts b/src/links/tests/update-internal-links-yaml.ts new file mode 100644 index 000000000000..05cd61bc63ca --- /dev/null +++ b/src/links/tests/update-internal-links-yaml.ts @@ -0,0 +1,184 @@ +import fs from 'fs' +import os from 'os' +import path from 'path' + +import { describe, expect, test } from 'vitest' +import { load } from 'js-yaml' + +import { serializeYaml, updateFile, type LinkContext } from '@/links/lib/update-internal-links' +import { RedirectedFragmentValidator } from '@/links/lib/validate-redirected-fragment' +import type { Page } from '@/types' + +const RELEASE_NOTE = `date: '2026-01-15' +sections: + bugs: + - | + See [AUTOTITLE](/admin/old-path) for details. + known_issues: + - | + Also see [AUTOTITLE](/admin/old-path) here. +` + +describe('serializeYaml', () => { + test('writes the surgically edited text when only links changed', () => { + const newContent = RELEASE_NOTE.replaceAll('/admin/old-path', '/admin/new-path') + + const result = serializeYaml( + newContent, + load(RELEASE_NOTE) as Record, + true, + false, + ) + + expect(result).toBe(newContent) + // Untouched YAML formatting must survive: block scalars, quoting, indentation. + expect(result).toContain("date: '2026-01-15'") + expect(result).toContain(' - |') + expect(result).not.toContain('/admin/old-path') + }) + + test('does not reserialize, so unrelated formatting is byte-identical', () => { + const result = serializeYaml( + RELEASE_NOTE, + load(RELEASE_NOTE) as Record, + false, + false, + ) + + expect(result).toBe(RELEASE_NOTE) + }) + + test('output still parses as YAML with the same structure', () => { + const newContent = RELEASE_NOTE.replaceAll('/admin/old-path', '/admin/new-path') + + const result = serializeYaml( + newContent, + load(RELEASE_NOTE) as Record, + true, + false, + ) + const parsed = load(result) as { sections: { bugs: string[]; known_issues: string[] } } + + expect(Object.keys(parsed)).toEqual(['date', 'sections']) + expect(parsed.sections.bugs[0]).toContain('/admin/new-path') + expect(parsed.sections.known_issues[0]).toContain('/admin/new-path') + }) + + test('reserializes when only the structured data changed', () => { + const result = serializeYaml( + RELEASE_NOTE, + { featuredLinks: { guide: '/new/path' } }, + false, + true, + ) + + expect(result).toContain('featuredLinks') + expect(result).toContain('/new/path') + }) + + // The text lives in `newContent` and the structured links live in `newData`, and there + // is no format-preserving way to merge them. Silently picking one loses the other. + test('throws rather than silently dropping fixes when both changed', () => { + expect(() => + serializeYaml(RELEASE_NOTE, { featuredLinks: { guide: '/new/path' } }, true, true), + ).toThrow(/both text and structured data changes/) + }) +}) + +describe('rewriting links in YAML', () => { + const context = { + pages: { '/en/admin/new-path': {} as unknown as Page }, + redirects: { '/admin/old-path': '/admin/new-path' }, + currentLanguage: 'en', + userLanguage: 'en', + fragmentValidator: new RedirectedFragmentValidator({}, {}, 'en'), + } as unknown as LinkContext + + const opts = { + setAutotitle: false, + fixHref: true, + verbose: false, + strict: false, + keepStaleFragments: false, + } + + async function run(yaml: string) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codemod-yaml-')) + const file = path.join(dir, 'notes.yml') + fs.writeFileSync(file, yaml) + try { + return await updateFile(file, context, opts) + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } + } + + test('rewrites a link hidden by indentation, which reads as a code block', async () => { + // Ten spaces of indent makes mdast see a code block, not a paragraph with a link. + const yaml = `sections: + bugs: + - | + See [x](/admin/old-path) here. + known_issues: + - | + See [x](/admin/old-path) here. +` + const result = await run(yaml) + expect(result.newContent).not.toContain('/admin/old-path') + expect(result.newContent!.match(/\/admin\/new-path/g)).toHaveLength(2) + expect(result.replacements).toHaveLength(2) + }) + + // A `#` comment reads as a Markdown heading, so a link inside one is a real link node + // and does get rewritten. That is a documented limit, not corruption: telling the two + // apart needs a YAML parse, and a stale link in a comment is worth fixing anyway. The + // cases that would be corruption, code examples, are covered below. + test('rewrites a link in a comment, but only there', async () => { + const yaml = `# TODO: drop [x](/admin/old-path) from the copy below +sections: + bugs: + - | + Keep \`[x](/admin/old-path)\` verbatim. +` + const result = await run(yaml) + expect(result.newContent).toContain('# TODO: drop [x](/admin/new-path) from the copy') + expect(result.newContent).toContain('Keep `[x](/admin/old-path)` verbatim.') + }) + + test('leaves an identical string inside inline code alone', async () => { + const yaml = `sections: + bugs: + - | + Write it as \`[x](/admin/old-path)\` and it renders as [x](/admin/old-path). +` + const result = await run(yaml) + expect(result.newContent).toContain('`[x](/admin/old-path)`') + expect(result.newContent).toContain('renders as [x](/admin/new-path).') + }) + + test('leaves an identical string inside a fenced code block alone', async () => { + const yaml = `sections: + bugs: + - | + Real link: [x](/admin/old-path) + + \`\`\`markdown + [x](/admin/old-path) + \`\`\` +` + const result = await run(yaml) + expect(result.newContent).toContain('Real link: [x](/admin/new-path)') + expect(result.newContent!.match(/\/admin\/old-path/g)).toHaveLength(1) + }) + + test('preserves every byte that is not a rewritten link', async () => { + const yaml = `date: '2026-01-15' +sections: + bugs: + - | + See [x](/admin/old-path) here. +` + const result = await run(yaml) + expect(result.newContent).toBe(yaml.replace('/admin/old-path', '/admin/new-path')) + }) +}) From ae976f4e7406247431c0f7c65ed45b3668578b67 Mon Sep 17 00:00:00 2001 From: Vanessa Date: Wed, 19 Aug 2026 19:43:18 +0000 Subject: [PATCH 8/8] Update "About support bundles" content expiry for commands and adjust note (#62737) Co-authored-by: Joe Clark <31087804+jc-clark@users.noreply.github.com> --- .../about-support-bundles.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/content/admin/monitoring-and-managing-your-instance/monitoring-your-instance/about-support-bundles.md b/content/admin/monitoring-and-managing-your-instance/monitoring-your-instance/about-support-bundles.md index 2539be2b3a56..1a20d28cd3c3 100644 --- a/content/admin/monitoring-and-managing-your-instance/monitoring-your-instance/about-support-bundles.md +++ b/content/admin/monitoring-and-managing-your-instance/monitoring-your-instance/about-support-bundles.md @@ -13,10 +13,10 @@ category: A support bundle is a compressed archive of diagnostic data from your {% data variables.product.prodname_ghe_server %} instance. You can use support bundles to work with {% data variables.contact.github_support %} on issues and to generate Health Check reports that summarize your instance's configuration, health, and activity. - + > [!IMPORTANT] -> Beginning August 18, 2026, the `ghe-support-bundle`, `ghe-cluster-support-bundle`, and `ghe-support-upload` commands require you to be on 3.21.3, 3.20.5, 3.19.9, 3.18.12, or 3.17.18 (or later). Please update your {% data variables.product.prodname_ghe_server %} instance to the latest patch for your version line before August 18, 2026. If you cannot update to the required patch version before August 18, 2026, and need to upload a support bundle, contact {% data variables.contact.contact_ent_server_support %} for guidance. - +> Use of the `ghe-support-bundle`, `ghe-cluster-support-bundle`, and `ghe-support-upload` commands require you to be on 3.21.3, 3.20.5, 3.19.9, 3.18.12, or 3.17.18 (or later). Please update your {% data variables.product.prodname_ghe_server %} instance to the latest patch for your version. If you cannot update to the required patch version and need to upload a support bundle, contact {% data variables.contact.contact_ent_server_support %} for guidance. + ## When to generate a support bundle @@ -123,10 +123,10 @@ The Monitor page in the {% data variables.enterprise.management_console %} provi ## Generating and sharing support bundles - + > [!IMPORTANT] -> Beginning August 18, 2026, the `ghe-support-bundle`, `ghe-cluster-support-bundle`, and `ghe-support-upload` commands require you to be on 3.21.3, 3.20.5, 3.19.9, 3.18.12, or 3.17.18 (or later). Please update your {% data variables.product.prodname_ghe_server %} instance to the latest patch for your version line before August 18, 2026. If you cannot update to the required patch version before August 18, 2026, and need to upload a support bundle, contact {% data variables.contact.contact_ent_server_support %} for guidance. - +> Use of the `ghe-support-bundle`, `ghe-cluster-support-bundle`, and `ghe-support-upload` commands require you to be on 3.21.3, 3.20.5, 3.19.9, 3.18.12, or 3.17.18 (or later). Please update your {% data variables.product.prodname_ghe_server %} instance to the latest patch for your version. If you cannot update to the required patch version and need to upload a support bundle, contact {% data variables.contact.contact_ent_server_support %} for guidance. + You can generate and share support bundles using the {% data variables.enterprise.management_console %} or the command line. For detailed instructions, see [AUTOTITLE](/support/contacting-github-support/providing-data-to-github-support#creating-and-sharing-support-bundles).