diff --git a/astro.config.ts b/astro.config.ts index 50af23649c..b871cf6cee 100644 --- a/astro.config.ts +++ b/astro.config.ts @@ -16,6 +16,7 @@ import remarkSmartypants from 'remark-smartypants'; import { asideAutoImport, astroAsides } from './integrations/astro-asides'; import { astroYoutubeEmbeds, youtubeAutoImport } from './integrations/astro-youtube-embed'; import { PagefindIndex } from './integrations/pagefind-index'; +import { validateApiNavPaths } from './integrations/validate-api-nav-paths'; import { validateDataTypeAnchors } from './integrations/validate-data-type-anchors'; import { autolinkConfig } from './plugins/rehype-autolink-config'; import { rehypeOptimizeStatic } from './plugins/rehype-optimize-static'; @@ -54,6 +55,7 @@ export default defineConfig({ }), PagefindIndex(), validateDataTypeAnchors(), + validateApiNavPaths(), icon({ include: { lucide: ['*'], diff --git a/integrations/validate-api-nav-paths.ts b/integrations/validate-api-nav-paths.ts new file mode 100644 index 0000000000..7939d80f49 --- /dev/null +++ b/integrations/validate-api-nav-paths.ts @@ -0,0 +1,39 @@ +import type { AstroIntegration } from 'astro'; +import rawApiSchema from '../public/api-schemas.json'; +import { type OpenAPISpec, preprocessSchema } from '../src/components/ApiReference/openapi'; +import { danglingApiNavPaths } from '../src/util/apiNavPaths'; + +/** + * Fail the build when the sidebar points at an `/api/*` page that no longer + * exists. Those pages are generated from the OpenAPI spec's tags, so renaming + * a tag silently deletes a route — which is how the `eventlogs` → + * `activity_log` rename left `/api/eventlogs` 404ing from every page on the + * site. + * + * This has to run at build time rather than only in the link check: the spec + * arrives by bot sync pushed straight to main, which opens no pull request, + * and CI runs on `pull_request` only. The deploy build is the sole gate a + * sync passes through. apiNavPaths.test.ts runs the same check in pull + * request CI for earlier feedback on docs-side edits. + */ +export function validateApiNavPaths(): AstroIntegration { + return { + name: 'validate-api-nav-paths', + hooks: { + 'astro:build:start': () => { + const dangling = danglingApiNavPaths( + preprocessSchema(rawApiSchema as unknown as OpenAPISpec) + ); + + if (dangling.length > 0) { + throw new Error( + `src/content/navItems.tsx links to ${dangling.join(', ')}, which no longer ` + + `exist. Every /api/* route comes from a tag in public/api-schemas.json or a ` + + `page in src/content/docs/api/ — point the entry at the tag's current slug ` + + `(and add a redirect in public/_redirects, since the old URL was published).` + ); + } + }, + }, + }; +} diff --git a/linkinator.config.json b/linkinator.config.json deleted file mode 100644 index 08ecc1fd6c..0000000000 --- a/linkinator.config.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "skip": [ - "^https?://(?:www\\.)?linkedin\\.com", - "^https?://(?:www\\.)?twitter\\.com", - "^https?://(?:www\\.)?x\\.com", - "^https://docs\\.mergify\\.com", - "^/api" - ] -} diff --git a/linkinator.config.mjs b/linkinator.config.mjs new file mode 100644 index 0000000000..1f727361f0 --- /dev/null +++ b/linkinator.config.mjs @@ -0,0 +1,50 @@ +/** + * Config for `pnpm check:links`. + * + * Kept as `.mjs` rather than the `.json` linkinator picks up by default so + * `EXTERNAL_LINK` can carry this comment and be unit-tested + * (linkinator.config.test.mjs). Both matter: the rule below has a silent + * failure mode that disabled the whole check for months. + */ + +/** + * Skip every off-site link, so CI never fails on someone else's outage or + * bot wall. Only links within the built site are verified. + * + * The negative lookahead is load-bearing. `linkinator dist/` serves the build + * from a local HTTP server and crawls it, so the pages under test are + * themselves `http://` URLs: a bare `https?://` rule matches the crawl root + * and linkinator exits happily having scanned zero links. That is precisely + * what the previous `--skip 'https?://'` did, which is why a sidebar link to a + * deleted `/api/*` page shipped green. + * + * Both hosts have to be excluded: linkinator binds the server to `127.0.0.1` + * (the crawl root) but builds its trailing-slash redirects against + * `localhost`. Excluding only one leaves every directory-style URL — nearly + * every page on the site — skipped at the redirect hop. + */ +export const EXTERNAL_LINK = '^https?://(?!(?:127\\.0\\.0\\.1|localhost)[:/])'; + +export default { + recurse: true, + verbosity: 'error', + skip: [EXTERNAL_LINK], + + /** + * linkinator crawls the built site through a static server it runs itself, + * and that server drops the occasional connection when several hundred + * pages are pulled at once — surfacing as a status-0 "broken" link on a + * different file each run. Back the concurrency off and retry those. + * + * `retryErrors` only covers status 0, 5xx and 429; a 404 is never retried, + * so this buys reliability without softening the check that matters. + * + * `retryErrorsCount` and `retryErrorsJitter` are deliberately absent: meow + * declares defaults for both, and a flag with a default is never `undefined` + * for linkinator's config merge to strip, so a value set here would be + * silently overridden by the built-in 5 and 3000ms. Set them on the command + * line if they ever need changing. + */ + concurrency: 10, + retryErrors: true, +}; diff --git a/linkinator.config.test.mjs b/linkinator.config.test.mjs new file mode 100644 index 0000000000..e8e7dececa --- /dev/null +++ b/linkinator.config.test.mjs @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import config, { EXTERNAL_LINK } from './linkinator.config.mjs'; + +/** + * linkinator applies `skip` with `new RegExp(rule).test(url)` against the + * absolute URL of every link it is about to fetch — including the crawl root + * and every redirect target. A rule that over-matches does not fail the build, + * it silently shrinks the crawl, so these cases are the only thing standing + * between us and a link check that scans nothing. + */ +describe('EXTERNAL_LINK', () => { + const skips = (url) => new RegExp(EXTERNAL_LINK).test(url); + + it('skips off-site links', () => { + expect(skips('https://github.com/Mergifyio')).toBe(true); + expect(skips('http://example.com/whatever')).toBe(true); + expect(skips('https://docs.mergify.com/api/activity-log')).toBe(true); + }); + + it('keeps the local crawl root, which linkinator binds to 127.0.0.1', () => { + expect(skips('http://127.0.0.1:3000/')).toBe(false); + expect(skips('http://127.0.0.1:3000/api/activity-log')).toBe(false); + }); + + it('keeps redirect targets, which linkinator builds against localhost', () => { + expect(skips('http://localhost:3000/api/activity-log/')).toBe(false); + expect(skips('http://localhost:3000/')).toBe(false); + }); + + // Without the trailing `[:/]` these would be treated as our own server. + it('does not mistake a look-alike host for the local server', () => { + expect(skips('https://localhost.example.com/')).toBe(true); + expect(skips('https://127.0.0.1.example.com/')).toBe(true); + }); + + // linkinator splits every skip rule on /[\s,]+/ and compiles the shards + // separately, so a rule carrying either would silently become a different, + // broader rule (and possibly an invalid one). + it('survives the split linkinator applies to skip rules', () => { + expect(EXTERNAL_LINK.split(/[\s,]+/)).toEqual([EXTERNAL_LINK]); + }); +}); + +describe('the exported config', () => { + // `recurse` lives only here now that the npm script passes no flags but + // --server-root and --config: drop it and the crawl silently becomes one page. + it('recurses, or the crawl never leaves the entry points', () => { + expect(config.recurse).toBe(true); + }); + + it('skips using the rule asserted above', () => { + expect(config.skip).toEqual([EXTERNAL_LINK]); + }); + + // meow declares defaults for these two, and a flag holding a default is + // never stripped from the merge, so a value set here would never apply. + it('omits the retry knobs that config cannot actually set', () => { + expect(config.retryErrors).toBe(true); + expect(config).not.toHaveProperty('retryErrorsCount'); + expect(config).not.toHaveProperty('retryErrorsJitter'); + }); +}); diff --git a/package.json b/package.json index 1efd7dccd0..5db4353e3d 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "check": "astro check && eslint . && biome check .", "check:config-examples": "node scripts/validate-config-examples.mjs", "check:internal-leaks": "node scripts/check-internal-leaks.mjs", - "check:links": "linkinator dist/ --recurse --concurrency 25 --verbosity error --skip 'https?://'" + "check:links": "linkinator / enterprise/ --server-root dist --config linkinator.config.mjs" }, "devDependencies": { "@actions/core": "^3.0.1", diff --git a/public/_redirects b/public/_redirects index 603c19027f..28155f886e 100644 --- a/public/_redirects +++ b/public/_redirects @@ -11,6 +11,8 @@ /api-intro/ /api/usage 301 /api-usage /api/usage 301 /api-usage/ /api/usage 301 +/api/eventlogs /api/activity-log/ 301 +/api/eventlogs/ /api/activity-log/ 301 /examples /integrations 301 /examples/ /integrations 301 /examples/_ /integrations/:splat 301 diff --git a/src/components/ApiReference/openapi.ts b/src/components/ApiReference/openapi.ts index 542860ff1b..772388d2fe 100644 --- a/src/components/ApiReference/openapi.ts +++ b/src/components/ApiReference/openapi.ts @@ -200,7 +200,7 @@ export const TAG_LABELS: Record = { queues: 'Queues', badges: 'Badges', simulator: 'Simulator', - eventlogs: 'Event Logs', + activity_log: 'Activity Log', statistics: 'Statistics', scheduled_freeze: 'Scheduled Freeze', merge_queue: 'Merge Queue', @@ -215,7 +215,7 @@ export const TAG_DESCRIPTIONS: Record = { queues: 'Configure and inspect merge queues for your repositories.', badges: 'Generate status badges for your repositories.', simulator: 'Simulate Mergify behavior on pull requests and configurations.', - eventlogs: 'Retrieve event logs for pull request activity.', + activity_log: 'Retrieve and aggregate the timeline of Mergify events for a repository.', statistics: 'Access merge queue and CI performance statistics.', scheduled_freeze: 'Schedule merge queue freezes for maintenance or release windows.', merge_queue: 'Control merge queue state — pause, unpause, and inspect status.', diff --git a/src/content/navItems.tsx b/src/content/navItems.tsx index 9d787ffb3b..cc51526874 100644 --- a/src/content/navItems.tsx +++ b/src/content/navItems.tsx @@ -335,7 +335,7 @@ const navItems: NavItem[] = [ { title: 'Merge Queue', path: '/api/merge-queue' }, { title: 'Statistics', path: '/api/statistics' }, { title: 'Simulator', path: '/api/simulator' }, - { title: 'Event Logs', path: '/api/eventlogs' }, + { title: 'Activity Log', path: '/api/activity-log' }, { title: 'Badges', path: '/api/badges' }, { title: 'Scheduled Freeze', path: '/api/scheduled-freeze' }, { title: 'CI Insights', path: '/api/ci-insights' }, diff --git a/src/util/apiNavPaths.test.ts b/src/util/apiNavPaths.test.ts new file mode 100644 index 0000000000..855f5534a7 --- /dev/null +++ b/src/util/apiNavPaths.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest'; +import type { OpenAPISpec } from '~/components/ApiReference/openapi'; +import apiSchema from '../../public/api-schemas.json'; +import { danglingApiNavPaths } from './apiNavPaths'; + +const schema = apiSchema as unknown as OpenAPISpec; + +/** The spec with `from` renamed to `to` wherever an operation is tagged with it. */ +function renameTag(from: string, to: string): OpenAPISpec { + const renamed = structuredClone(schema); + for (const pathItem of Object.values(renamed.paths)) { + for (const operation of Object.values(pathItem)) { + if (Array.isArray(operation?.tags)) { + operation.tags = operation.tags.map((tag) => (tag === from ? to : tag)); + } + } + } + return renamed; +} + +describe('danglingApiNavPaths', () => { + it('accepts the sidebar as it stands', () => { + expect(danglingApiNavPaths(schema)).toEqual([]); + }); + + // The regression: a tag rename in a synced spec deletes the route the + // sidebar points at, and every page of the site carries that link. + it('reports a nav path whose tag is gone from the spec', () => { + expect(danglingApiNavPaths(renameTag('activity_log', 'eventlogs'))).toEqual([ + '/api/activity-log', + ]); + }); + + // /api/usage is src/content/docs/api/usage.mdx. It has no tag behind it, so + // it is only ever accepted by the hand-written half of the check — which + // makes this the case that fails if that half breaks. + it('accepts a hand-written page with no tag behind it', () => { + expect(danglingApiNavPaths(renameTag('activity_log', 'eventlogs'))).not.toContain('/api/usage'); + // Every tag gone: only the hand-written page and the index may survive. + const untagged = structuredClone(schema); + for (const pathItem of Object.values(untagged.paths)) { + for (const operation of Object.values(pathItem)) { + if (Array.isArray(operation?.tags)) operation.tags = []; + } + } + expect(danglingApiNavPaths(untagged)).not.toContain('/api/usage'); + }); + + // navItems already ships anchored entries elsewhere + // (/test-insights#test-framework-configuration), and the /api pages are the + // ones with a heading per endpoint, so anchoring one is the natural next + // edit. Reading the anchor as part of the slug would fail the deploy build + // on a link that resolves. + it('resolves an anchored or queried nav path against the page it points at', () => { + const nav = [ + { title: 'Anchored tag page', path: '/api/activity-log#list-events' }, + { title: 'Anchored hand-written page', path: '/api/usage?utm=nav' }, + ]; + expect(danglingApiNavPaths(schema, nav)).toEqual([]); + }); + + it('accepts both spellings of the reference index', () => { + const nav = [ + { title: 'Reference', path: '/api/' }, + { title: 'Reference, no slash', path: '/api' }, + ]; + expect(danglingApiNavPaths(schema, nav)).toEqual([]); + }); +}); diff --git a/src/util/apiNavPaths.ts b/src/util/apiNavPaths.ts new file mode 100644 index 0000000000..d47e61d1d0 --- /dev/null +++ b/src/util/apiNavPaths.ts @@ -0,0 +1,53 @@ +// Node-only: reads the hand-written /api pages from disk. Kept out of +// navItems.tsx itself so components can keep importing the nav tree without +// dragging node:fs into a client bundle. +import { readdirSync } from 'node:fs'; +import { groupByTag, type OpenAPISpec, slugifyTag } from '~/components/ApiReference/openapi'; +import navItems, { type NavItem } from '~/content/navItems'; +import { flattenNavItems } from '~/util/flattenNavItems'; + +const API_DOCS_URL = new URL('../content/docs/api/', import.meta.url); + +/** + * The slugs of the pages written by hand under `src/content/docs/api/`, + * mirroring the glob the docs collection loads them with + * (src/content.config.ts): recursive, and `_`-prefixed partials excluded, + * because those never become routes. + */ +function handwrittenApiPages(): Set { + return new Set( + readdirSync(API_DOCS_URL, { recursive: true, encoding: 'utf8' }) + .filter((name) => name.endsWith('.mdx')) + .map((name) => name.replace(/\.mdx$/, '')) + .filter((slug) => !slug.split('/').some((segment) => segment.startsWith('_'))) + ); +} + +/** + * The `/api/*` sidebar links that resolve to nothing. + * + * Everything under `/api/` is either generated from an OpenAPI tag by + * `src/pages/api/[tag].astro` — handed the same preprocessed schema here, and + * grouped and slugified by the same helpers, so the two cannot disagree about + * which routes exist — or a page written by hand under + * `src/content/docs/api/`. A path matching neither is a 404 on every page of + * the site, since the sidebar renders site-wide. + */ +export function danglingApiNavPaths(schema: OpenAPISpec, items: NavItem[] = navItems): string[] { + const generated = new Set([...groupByTag(schema).keys()].map(slugifyTag)); + const handwritten = handwrittenApiPages(); + + const dangling = flattenNavItems(items) + .flatMap((item) => (item.path && /^\/api(\/|$)/.test(item.path) ? [item.path] : [])) + .filter((path) => { + // A nav path may carry an anchor or query onto an otherwise real page. + const slug = path + .replace(/[#?].*$/, '') + .replace(/^\/api\/?/, '') + .replace(/\/$/, ''); + // `/api` itself is the reference index, src/pages/api/index.astro. + if (slug === '') return false; + return !generated.has(slug) && !handwritten.has(slug); + }); + return [...new Set(dangling)].sort(); +}