Skip to content

Commit 7829a05

Browse files
authored
Fix link checker: validate assets on disk, compact redirects, no alerts (#59358)
1 parent d371058 commit 7829a05

File tree

7 files changed

+97
-12
lines changed

7 files changed

+97
-12
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,13 @@ jobs:
8989
run: npm run check-links-internal
9090

9191
- name: Upload report artifact
92-
if: failure()
92+
if: always()
9393
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
9494
with:
9595
name: link-report-${{ matrix.version }}-${{ matrix.language }}
9696
path: artifacts/link-report-*.md
9797
retention-days: 5
98+
if-no-files-found: ignore
9899

99100
- uses: ./.github/actions/slack-alert
100101
if: ${{ failure() && github.event_name != 'workflow_dispatch' }}

src/links/lib/extract-links.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,21 @@ export function checkInternalLink(
337337

338338
return { exists: false, isRedirect: false }
339339
}
340+
341+
/**
342+
* Check if an asset link points to an existing file on disk
343+
*/
344+
export function checkAssetLink(href: string): boolean {
345+
if (!href.startsWith('/assets/')) {
346+
return false // Not an asset link
347+
}
348+
const assetPath = path.resolve(href.slice(1)) // Remove leading /
349+
return fs.existsSync(assetPath)
350+
}
351+
352+
/**
353+
* Check if a link is an asset link (starts with /assets/)
354+
*/
355+
export function isAssetLink(href: string): boolean {
356+
return href.startsWith('/assets/')
357+
}

src/links/lib/link-report.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,7 @@ ${errors
122122

123123
const warningSection =
124124
warnings.length > 0
125-
? `### ⚠️ ${warnings.length} Redirect${warnings.length === 1 ? '' : 's'} to Update
126-
127-
${warnings.map((group) => `- \`${group.target}\` → \`${group.occurrences[0]?.redirectTarget || '?'}\``).join('\n')}
125+
? `### ℹ️ ${warnings.length} redirect${warnings.length === 1 ? '' : 's'} to update
128126
129127
`
130128
: ''

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ import warmServer from '@/frame/lib/warm-server'
2626
import { renderContent } from '@/content-render/index'
2727
import { allVersions, allVersionKeys } from '@/versions/lib/all-versions'
2828
import languages from '@/languages/lib/languages-server'
29-
import { normalizeLinkPath, checkInternalLink } from '@/links/lib/extract-links'
29+
import {
30+
normalizeLinkPath,
31+
checkInternalLink,
32+
checkAssetLink,
33+
isAssetLink,
34+
} from '@/links/lib/extract-links'
3035
import {
3136
type BrokenLink,
3237
generateInternalLinkReport,
@@ -183,6 +188,19 @@ async function checkVersion(
183188
for (const link of links) {
184189
if (isExcludedLink(link.href)) continue
185190

191+
// Check if this is an asset link (images, etc.) - verify file exists on disk
192+
if (isAssetLink(link.href)) {
193+
if (!checkAssetLink(link.href)) {
194+
brokenLinks.push({
195+
href: link.href,
196+
file: page.relativePath,
197+
lines: [0],
198+
text: link.text,
199+
})
200+
}
201+
continue
202+
}
203+
186204
const normalized = normalizeLinkPath(link.href)
187205
const result = checkInternalLink(normalized, pageMap, redirects)
188206

@@ -346,10 +364,14 @@ async function main() {
346364
console.log(`Created report issue: ${newReport.html_url}`)
347365
}
348366

349-
// Exit with error if broken links found
350-
if (result.brokenLinks.length > 0) {
351-
process.exit(1)
352-
}
367+
// Don't exit with error - the issue report is the mechanism for docs-content to act on broken links
368+
// Exiting with error would trigger docs-alerts which only engineering monitors
369+
console.log('')
370+
console.log(
371+
chalk.yellow(
372+
'Note: Report generated. Broken links should be fixed via the issue created in docs-content.',
373+
),
374+
)
353375
}
354376

355377
// Run if invoked directly

src/links/scripts/check-links-pr.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import {
2424
extractLinksWithLiquid,
2525
createLiquidContext,
2626
checkInternalLink,
27+
checkAssetLink,
28+
isAssetLink,
2729
getRelativePath,
2830
} from '@/links/lib/extract-links'
2931
import { type BrokenLink, generatePRComment, groupBrokenLinks } from '@/links/lib/link-report'
@@ -74,6 +76,20 @@ async function checkFile(
7476
totalLinksChecked = internalLinks.length
7577

7678
for (const link of internalLinks) {
79+
// Check if this is an asset link (images, etc.) - verify file exists on disk
80+
if (isAssetLink(link.href)) {
81+
if (!checkAssetLink(link.href)) {
82+
brokenLinks.push({
83+
href: link.href,
84+
file: getRelativePath(filePath),
85+
lines: [link.line],
86+
text: link.text,
87+
isAutotitle: link.isAutotitle,
88+
})
89+
}
90+
continue
91+
}
92+
7793
const result = checkInternalLink(link.href, pageMap, redirects)
7894

7995
if (!result.exists) {

src/links/tests/extract-links.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {
33
extractLinksFromMarkdown,
44
normalizeLinkPath,
55
checkInternalLink,
6+
checkAssetLink,
7+
isAssetLink,
68
} from '../lib/extract-links'
79

810
describe('extractLinksFromMarkdown', () => {
@@ -209,3 +211,31 @@ describe('checkInternalLink', () => {
209211
expect(result.isRedirect).toBe(false)
210212
})
211213
})
214+
215+
describe('isAssetLink', () => {
216+
test('returns true for asset paths', () => {
217+
expect(isAssetLink('/assets/images/help/test.png')).toBe(true)
218+
expect(isAssetLink('/assets/cb-12345/images/help/test.png')).toBe(true)
219+
})
220+
221+
test('returns false for non-asset paths', () => {
222+
expect(isAssetLink('/actions/getting-started')).toBe(false)
223+
expect(isAssetLink('/repositories/overview')).toBe(false)
224+
expect(isAssetLink('https://example.com/assets/image.png')).toBe(false)
225+
})
226+
})
227+
228+
describe('checkAssetLink', () => {
229+
test('returns true for existing asset files', () => {
230+
// Use a known existing asset file
231+
expect(checkAssetLink('/assets/images/help/writing/headings-rendered.png')).toBe(true)
232+
})
233+
234+
test('returns false for non-existent asset files', () => {
235+
expect(checkAssetLink('/assets/images/does-not-exist-12345.png')).toBe(false)
236+
})
237+
238+
test('returns false for non-asset paths', () => {
239+
expect(checkAssetLink('/actions/getting-started')).toBe(false)
240+
})
241+
})

src/links/tests/link-report.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,9 @@ describe('generatePRComment', () => {
269269

270270
const comment = generatePRComment(links)
271271

272-
expect(comment).toContain('Redirect')
273-
expect(comment).toContain('`/old`')
274-
expect(comment).toContain('`/new`')
272+
// Redirects now show a compact summary with info icon
273+
expect(comment).toContain('redirect')
274+
expect(comment).toContain('ℹ️')
275275
})
276276

277277
test('limits occurrences to 3 per group', () => {

0 commit comments

Comments
 (0)