diff --git a/.github/workflows/benchmark-pages.yml b/.github/workflows/benchmark-pages.yml index aa0e0edf696a..cb72747b0bb9 100644 --- a/.github/workflows/benchmark-pages.yml +++ b/.github/workflows/benchmark-pages.yml @@ -51,6 +51,11 @@ jobs: RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | echo "Reading benchmark results..." + # The Run step pipes through tee, so a crashed benchmark passes that step and leaves no results file. + if [ ! -s /tmp/benchmark-results.json ]; then + echo "::error::No benchmark results file. The benchmark did not finish, so no issue was closed." + exit 1 + fi ERRORS=$(jq '.errors | length' /tmp/benchmark-results.json 2>/dev/null || echo "0") SLOW=$(jq '.slow | length' /tmp/benchmark-results.json 2>/dev/null || echo "0") TOTAL=$(jq '.totalRequests' /tmp/benchmark-results.json 2>/dev/null || echo "0") @@ -58,6 +63,10 @@ jobs: P99=$(jq '.p99' /tmp/benchmark-results.json 2>/dev/null || echo "0") MAX=$(jq '.max' /tmp/benchmark-results.json 2>/dev/null || echo "0") echo "Done reading results: $TOTAL pages, $ERRORS errors, $SLOW slow" + if [ "$TOTAL" = "0" ] || [ "$TOTAL" = "null" ]; then + echo "::error::Benchmark ran 0 requests, so no issue was closed." + exit 1 + fi VERSIONS="free-pro-team@latest, enterprise-cloud@latest, enterprise-server@latest" LANGS="en" diff --git a/.github/workflows/headless-tests.yml b/.github/workflows/headless-tests.yml index a000570dadad..0f5192e5ec96 100644 --- a/.github/workflows/headless-tests.yml +++ b/.github/workflows/headless-tests.yml @@ -27,6 +27,7 @@ jobs: matrix: node: - playwright-rendering + - playwright-header - playwright-a11y - playwright-secret-scanning fail-fast: false diff --git a/.github/workflows/orphaned-files-check.yml b/.github/workflows/orphaned-files-check.yml index 3dc7fdbfdfbe..5cbbf71b4a2b 100644 --- a/.github/workflows/orphaned-files-check.yml +++ b/.github/workflows/orphaned-files-check.yml @@ -88,7 +88,7 @@ jobs: git push origin $branchname body=$(cat <<-EOM - Found with the `npm run find-orphaned-assets`, `npm run -s reusables -- find unused`, and `npm run find-orphaned-tables` scripts. + Found with the \`npm run find-orphaned-assets\`, \`npm run -s reusables -- find unused\`, and \`npm run find-orphaned-tables\` scripts. The orphaned files workflow file .github/workflows/orphaned-files-check.yml runs every Monday at 16:20 UTC / 8:20 PST. diff --git a/.github/workflows/sync-sdk-docs.yml b/.github/workflows/sync-sdk-docs.yml index 21587446765b..4858762e2631 100644 --- a/.github/workflows/sync-sdk-docs.yml +++ b/.github/workflows/sync-sdk-docs.yml @@ -53,7 +53,7 @@ jobs: # Publishing runs must start from the default branch because preserve-redirects.ts reads # live URLs from git HEAD. workflow_dispatch from another branch could write stale redirects. - name: Verify publishing runs start from the default branch - if: github.event_name != 'pull_request' && inputs.dry_run != 'true' + if: github.event_name != 'pull_request' && inputs.dry_run != true run: | if [ "$GITHUB_REF_NAME" != "$DEFAULT_BRANCH" ]; then echo "::error::This run would push and open a PR, but it started from" \ @@ -192,7 +192,7 @@ jobs: if: >- env.has_changes == 'true' && github.event_name != 'pull_request' - && inputs.dry_run != 'true' + && inputs.dry_run != true env: GH_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }} run: | @@ -215,7 +215,7 @@ jobs: if: >- env.has_changes == 'true' && github.event_name != 'pull_request' - && inputs.dry_run != 'true' + && inputs.dry_run != true env: GH_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }} run: | diff --git a/Dockerfile b/Dockerfile index 873b06c422cd..fb5b15f2b161 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ # Update the base image digest from the gh-base-noble package page: # https://github.com/github/gh-base-image/pkgs/container/gh-base-image%2Fgh-base-noble -FROM ghcr.io/github/gh-base-image/gh-base-noble:20261007-171807-gaf2071665@sha256:57e57e0d0dd8cf12cbf047aa5ae7ead850f562bc66bace621598a460b803efc7 AS base +FROM ghcr.io/github/gh-base-image/gh-base-noble:20261016-181622-g7dfd3798b@sha256:d41c754579434b9b6479a3d3786a6a84ad7be50ce64626bbc07579ab4c36302b AS base # Install curl for NodeSource setup. # Install git for early-access and translation clones. # Ubuntu's nodejs package lags the Node LTS release line. @@ -79,7 +79,7 @@ COPY --chown=node:node --from=clones $APP_HOME/content content/ COPY --chown=node:node --from=clones $APP_HOME/translations translations/ COPY --chown=node:node --from=prod_deps $APP_HOME/node_modules node_modules/ COPY --chown=node:node --from=build $APP_HOME/.next .next/ -COPY --chown=node:node --from=warmup_cache $APP_HOME/.remotejson-cache ./ +COPY --chown=node:node --from=warmup_cache $APP_HOME/.remotejson-cache .remotejson-cache/ # Expose the build SHA at runtime when callers pass --build-arg BUILD_SHA=abc123. ARG BUILD_SHA ENV BUILD_SHA=$BUILD_SHA diff --git a/content/copilot/tutorials/customization-library/index.md b/content/copilot/tutorials/customization-library/index.md index a9eb2a2b77db..4a53af412459 100644 --- a/content/copilot/tutorials/customization-library/index.md +++ b/content/copilot/tutorials/customization-library/index.md @@ -13,7 +13,7 @@ spotlight: image: /assets/images/copilot-landing/generating_unit_tests.png - article: /prompt-files/your-first-prompt-file image: /assets/images/copilot-landing/improving_code_readability.png - - article: /custom-instructions/your-first-custom-agent + - article: /custom-agents/your-first-custom-agent image: /assets/images/copilot-landing/generating_unit_tests.png filters: - category diff --git a/src/article-api/lib/get-all-toc-items.ts b/src/article-api/lib/get-all-toc-items.ts index 404a5b1ec905..d4e8ca5bc7db 100644 --- a/src/article-api/lib/get-all-toc-items.ts +++ b/src/article-api/lib/get-all-toc-items.ts @@ -48,7 +48,10 @@ export async function getAllTocItems( })) .filter( (entry): entry is { childHref: string; childPage: PageWithChildren } => - entry.childPage !== undefined, + entry.childPage !== undefined && + // resolvePath can return a page from another version, which the website hides. + (!context.currentVersion || + entry.childPage.applicableVersions.includes(context.currentVersion)), ) const items = await Promise.all( diff --git a/src/article-api/lib/get-link-data.ts b/src/article-api/lib/get-link-data.ts index de2d1955297b..506ba5956174 100644 --- a/src/article-api/lib/get-link-data.ts +++ b/src/article-api/lib/get-link-data.ts @@ -12,9 +12,13 @@ export async function getLinkData( pathname: string, context: Context, ) => Page | undefined, -): Promise { +): Promise { const linkedPage = resolvePath(href, languageCode, pathname, context) - if (!linkedPage) return { href, title: href } + if (!linkedPage) return null + // resolvePath can return a page from another version, which the website hides. + if (context.currentVersion && !linkedPage.applicableVersions.includes(context.currentVersion)) { + return null + } const title = await linkedPage.renderTitle(context, { unwrap: true }) const intro = linkedPage.intro diff --git a/src/article-api/lib/resolve-carousel-links.ts b/src/article-api/lib/resolve-carousel-links.ts new file mode 100644 index 000000000000..ad7741ba56ac --- /dev/null +++ b/src/article-api/lib/resolve-carousel-links.ts @@ -0,0 +1,35 @@ +import path from 'path' + +import type { Context, Page } from '@/types' +import type { LinkData } from '@/article-api/transformers/types' + +// Match resolve-carousels and GHD056: try the content root first, then the landing page's directory. +export async function resolveCarouselLinks( + rawPaths: string[], + page: Page, + context: Context, +): Promise { + const { default: getPageLinkData } = await import('@/frame/lib/get-link-data') + const options = { title: true, intro: true } + const pageDir = page.relativePath ? path.posix.dirname(page.relativePath) : '.' + + const links: LinkData[] = [] + for (const rawPath of rawPaths) { + let linkData = await getPageLinkData(rawPath, context, options) + // Liquid can render to an empty path, which would resolve to the landing page itself. + if (!linkData?.length && pageDir !== '.' && !rawPath.includes('{')) { + linkData = await getPageLinkData(path.posix.join('/', pageDir, rawPath), context, options) + } + for (const item of linkData || []) { + // Versionless redirects can resolve to a page that does not exist in the requested version. + if ( + context.currentVersion && + !item.page.applicableVersions.includes(context.currentVersion) + ) { + continue + } + links.push({ href: item.href, title: item.title || '', intro: item.intro || '' }) + } + } + return links +} diff --git a/src/article-api/lib/resolve-path.ts b/src/article-api/lib/resolve-path.ts index 593aea61634f..e68e61393b0f 100644 --- a/src/article-api/lib/resolve-path.ts +++ b/src/article-api/lib/resolve-path.ts @@ -17,7 +17,17 @@ export function resolvePath( const page = findPage(candidate, pages, redirects) || findPage(candidate.replace(/\/?$/, '/'), pages, redirects) - if (page) return page + if (page) { + // Content-root candidates span every version, so drop pages the web page would omit. + if ( + href.startsWith('/content/') && + context.currentVersion && + !page.applicableVersions?.includes(context.currentVersion) + ) { + return undefined + } + return page + } } return undefined @@ -28,6 +38,17 @@ function* candidates(href: string, lang: string, pathname: string) { const langPrefix = `/${lang}/` const cleanPathname = pathname.replace(/\/$/, '') + // Children like /content/actions/workflows point at the content root, matching create-tree.ts. + if (href.startsWith('/content/')) { + const rest = href.slice('/content/'.length).replace(/\/$/, '') + if (!rest) return + yield `${langPrefix}${rest}` + for (const version of allVersionKeys) { + yield `${langPrefix}${version}/${rest}` + } + return + } + if (href.startsWith(langPrefix)) { yield href } else if (href.startsWith('/')) { diff --git a/src/article-api/middleware/article-body.ts b/src/article-api/middleware/article-body.ts index 1dc44229defb..20192f2ad153 100644 --- a/src/article-api/middleware/article-body.ts +++ b/src/article-api/middleware/article-body.ts @@ -3,6 +3,7 @@ import type { Response } from 'express' import { Context } from '@/types' import { ExtendedRequestWithPageInfo } from '@/article-api/types' import contextualize from '@/frame/middleware/context/context' +import shortVersions from '@/versions/middleware/short-versions' import features from '@/versions/middleware/features' import glossaries from '@/frame/middleware/context/glossaries' import dataTables from '@/data-directory/middleware/data-tables' @@ -12,12 +13,18 @@ import { allVersions } from '@/versions/lib/all-versions' import type { Page } from '@/types' // Article Markdown rendering needs a mocked request with page context. -async function createContextualizedRenderingRequest(pathname: string, page: Page) { +async function createContextualizedRenderingRequest( + pathname: string, + page: Page, + apiVersion: string | undefined, +) { const mockedContext: Context = {} const renderingReq = { path: pathname, language: page.languageCode, pagePath: pathname, + // Content Liquid filters REST changelog sections on query.apiVersion. + query: apiVersion ? { apiVersion } : {}, cookies: {}, context: mockedContext, headers: { @@ -29,6 +36,9 @@ async function createContextualizedRenderingRequest(pathname: string, page: Page await contextualize(renderingReq as ExtendedRequestWithPageInfo, {} as Response, () => {}) renderingReq.context.page = page + // ifversion with bare fpt, ghec, or ghes reads short version flags from context. + await shortVersions(renderingReq as ExtendedRequestWithPageInfo, {} as Response, () => {}) + // ifversion Liquid tags read feature flags from context. features(renderingReq as ExtendedRequestWithPageInfo, {} as Response, () => {}) @@ -54,7 +64,7 @@ export async function getArticleBody(req: ExtendedRequestWithPageInfo) { const transformer = transformerRegistry.findTransformer(page) if (!transformer) throw new Error(`No transformer found for page: ${pathname}`) - const renderingReq = await createContextualizedRenderingRequest(pathname, page) + const renderingReq = await createContextualizedRenderingRequest(pathname, page, apiVersion) // apiVersionValidationMiddleware rejects invalid explicit API versions before body rendering. const currentVersion = renderingReq.context.currentVersion diff --git a/src/article-api/tests/article-body.ts b/src/article-api/tests/article-body.ts index 3ecf10cd6378..3e097f04f0ed 100644 --- a/src/article-api/tests/article-body.ts +++ b/src/article-api/tests/article-body.ts @@ -2,8 +2,8 @@ import { beforeAll, describe, expect, test } from 'vitest' import { get } from '@/tests/helpers/e2etest' -const makeURL = (pathname: string): string => - `/api/article/body?${new URLSearchParams({ pathname })}` +const makeURL = (pathname: string, apiVersion?: string): string => + `/api/article/body?${new URLSearchParams(apiVersion ? { pathname, apiVersion } : { pathname })}` describe('article body api', () => { beforeAll(() => { @@ -70,6 +70,41 @@ describe('article body api', () => { expect(res.body).toContain('Foo is an intentionally meaningless placeholder') }) + test('intro ifversion with a bare version name renders for the requested version', async () => { + { + const res = await get(makeURL('/en/get-started/liquid/ifversion')) + expect(res.statusCode).toBe(200) + expect(res.body).toContain('(bare not ghes)') + expect(res.body).not.toContain('(bare ghes)') + } + { + const res = await get(makeURL('/en/enterprise-server@latest/get-started/liquid/ifversion')) + expect(res.statusCode).toBe(200) + expect(res.body).toContain('(bare ghes)') + expect(res.body).not.toContain('(bare not ghes)') + } + }) + + test('apiVersion query filters Liquid that reads query.apiVersion', async () => { + const pathname = '/en/rest/about-the-rest-api/comparing-githubs-rest-api-and-graphql-api' + const marker = 'This paragraph applies to API version 2026-03-10 and later.' + { + const res = await get(makeURL(pathname)) + expect(res.statusCode).toBe(200) + expect(res.body).toContain(marker) + } + { + const res = await get(makeURL(pathname, '2026-03-10')) + expect(res.statusCode).toBe(200) + expect(res.body).toContain(marker) + } + { + const res = await get(makeURL(pathname, '2022-11-28')) + expect(res.statusCode).toBe(200) + expect(res.body).not.toContain(marker) + } + }) + test('invalid Referer header does not crash', async () => { const res = await get(makeURL('/en/get-started/start-your-journey/hello-world'), { headers: { diff --git a/src/article-api/tests/get-all-toc-items.ts b/src/article-api/tests/get-all-toc-items.ts new file mode 100644 index 000000000000..de877a40154e --- /dev/null +++ b/src/article-api/tests/get-all-toc-items.ts @@ -0,0 +1,64 @@ +import { describe, expect, test, vi } from 'vitest' +import { getAllTocItems } from '@/article-api/lib/get-all-toc-items' +import type { Context, Page, Permalink } from '@/types' + +function createMockPage(options: { + rawTitle: string + applicableVersions: string[] + permalinks: Partial[] + children?: string[] +}): Page { + return { + languageCode: 'en', + rawTitle: options.rawTitle, + applicableVersions: options.applicableVersions, + permalinks: options.permalinks as Permalink[], + children: options.children, + renderProp: vi.fn().mockResolvedValue(''), + } as unknown as Page +} + +describe('getAllTocItems', () => { + test('omits children that are not available in the current version', async () => { + const parent = createMockPage({ + rawTitle: 'REST API', + applicableVersions: ['free-pro-team@latest', 'enterprise-cloud@latest'], + permalinks: [{ languageCode: 'en', pageVersion: 'free-pro-team@latest', href: '/en/rest' }], + children: ['/quickstart', '/enterprise-admin'], + }) + const quickstart = createMockPage({ + rawTitle: 'Quickstart', + applicableVersions: ['free-pro-team@latest', 'enterprise-cloud@latest'], + permalinks: [ + { languageCode: 'en', pageVersion: 'free-pro-team@latest', href: '/en/rest/quickstart' }, + ], + }) + const enterpriseAdmin = createMockPage({ + rawTitle: 'Enterprise administration', + applicableVersions: ['enterprise-cloud@latest'], + permalinks: [ + { + languageCode: 'en', + pageVersion: 'enterprise-cloud@latest', + href: '/en/enterprise-cloud@latest/rest/enterprise-admin', + }, + ], + }) + const context = { + currentVersion: 'free-pro-team@latest', + pages: { + '/en/rest': parent, + '/en/rest/quickstart': quickstart, + '/en/enterprise-cloud@latest/rest/enterprise-admin': enterpriseAdmin, + }, + redirects: { + '/rest/enterprise-admin': '/enterprise-cloud@latest/rest/enterprise-admin', + }, + } as unknown as Context + + const items = await getAllTocItems(parent, context) + + expect(items.map((item) => item.href)).toEqual(['/en/rest/quickstart']) + expect(items[0].title).toBe('Quickstart') + }) +}) diff --git a/src/article-api/tests/get-link-data.ts b/src/article-api/tests/get-link-data.ts index 41f9a16b4e39..575f56c81a21 100644 --- a/src/article-api/tests/get-link-data.ts +++ b/src/article-api/tests/get-link-data.ts @@ -6,10 +6,12 @@ function createMockPage(options: { title?: string intro?: string permalinks?: Partial[] + applicableVersions?: string[] }): Page { const page = { title: options.title || 'Test Title', intro: options.intro, + applicableVersions: options.applicableVersions || ['free-pro-team@latest'], permalinks: (options.permalinks || []) as Permalink[], renderTitle: vi.fn().mockResolvedValue(options.title || 'Test Title'), renderProp: vi.fn().mockResolvedValue(options.intro || ''), @@ -23,7 +25,7 @@ function createContext(currentVersion = 'free-pro-team@latest'): Context { describe('getLinkData', () => { describe('when page is not found', () => { - test('returns href as both href and title when page not resolved', async () => { + test('returns null when page not resolved', async () => { const resolvePath = vi.fn().mockReturnValue(undefined) const context = createContext() @@ -35,10 +37,31 @@ describe('getLinkData', () => { resolvePath, ) - expect(result).toEqual({ - href: '/en/missing-page', - title: '/en/missing-page', + expect(result).toBeNull() + }) + }) + + describe('when page is not available in the current version', () => { + test('returns null', async () => { + const page = createMockPage({ + title: 'Public monitoring', + applicableVersions: ['free-pro-team@latest', 'enterprise-cloud@latest'], + permalinks: [ + { languageCode: 'en', pageVersion: 'free-pro-team@latest', href: '/en/page' }, + { + languageCode: 'en', + pageVersion: 'enterprise-cloud@latest', + href: '/en/enterprise-cloud@latest/page', + }, + ], }) + const resolvePath = vi.fn().mockReturnValue(page) + const context = createContext('enterprise-server@3.19') + + const result = await getLinkData('/page', 'en', '/en/current', context, resolvePath) + + expect(result).toBeNull() + expect(page.renderTitle).not.toHaveBeenCalled() }) }) @@ -50,7 +73,7 @@ describe('getLinkData', () => { const result = await getLinkData('/en/some-page', 'en', '/en/current', context, resolvePath) - expect(result.title).toBe('My Page Title') + expect(result?.title).toBe('My Page Title') expect(page.renderTitle).toHaveBeenCalledWith(context, { unwrap: true }) }) @@ -64,7 +87,7 @@ describe('getLinkData', () => { const result = await getLinkData('/en/some-page', 'en', '/en/current', context, resolvePath) - expect(result.intro).toBe('This is the intro text') + expect(result?.intro).toBe('This is the intro text') expect(page.renderProp).toHaveBeenCalledWith('intro', context, { textOnly: true }) }) @@ -75,7 +98,7 @@ describe('getLinkData', () => { const result = await getLinkData('/en/some-page', 'en', '/en/current', context, resolvePath) - expect(result.intro).toBe('') + expect(result?.intro).toBe('') expect(page.renderProp).not.toHaveBeenCalled() }) @@ -97,7 +120,7 @@ describe('getLinkData', () => { resolvePath, ) - expect(result.href).toBe('/en/resolved-path') + expect(result?.href).toBe('/en/resolved-path') }) test('falls back to original href when no matching permalink', async () => { @@ -116,7 +139,7 @@ describe('getLinkData', () => { resolvePath, ) - expect(result.href).toBe('/en/original-href') + expect(result?.href).toBe('/en/original-href') }) }) diff --git a/src/article-api/tests/landing-carousels.test.ts b/src/article-api/tests/landing-carousels.test.ts new file mode 100644 index 000000000000..f12faca587ac --- /dev/null +++ b/src/article-api/tests/landing-carousels.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, test, vi, beforeEach } from 'vitest' + +import getLinkData from '@/frame/lib/get-link-data' +import { DiscoveryLandingTransformer } from '@/article-api/transformers/discovery-landing-transformer' +import { BespokeLandingTransformer } from '@/article-api/transformers/bespoke-landing-transformer' +import type { Context, Page } from '@/types' +import type { TemplateData } from '@/article-api/transformers/types' + +// /b exists only under the landing page's directory, like the page-relative fixture entries. +// /cloud-only mimics a versionless redirect to a page the requested version does not have. +const pageVersions: Record = { + '/a': ['enterprise-server@3.19'], + '/landing/b': ['enterprise-server@3.19'], + '/cloud-only': ['enterprise-cloud@latest'], +} + +vi.mock('@/frame/lib/get-link-data', () => ({ + default: vi.fn(async (rawLink: string, context: Context) => + pageVersions[rawLink] + ? [ + { + href: `/${context.currentLanguage}/${context.currentVersion}${rawLink}`, + title: `Title for ${rawLink}`, + intro: `Intro for ${rawLink}`, + page: { applicableVersions: pageVersions[rawLink] }, + }, + ] + : undefined, + ), +})) + +vi.mock('@/content-render/index', () => ({ + renderContent: vi.fn((content: string) => content), +})) + +type PrepareTemplateData = (page: Page, pathname: string, context: Context) => Promise + +// resolve-carousels middleware leaves version-specific ResolvedArticle[] on the shared Page. +const makePage = (layout: string, recommended: string[] = ['/a']) => + ({ + layout, + relativePath: 'landing/index.md', + title: 'Landing', + intro: '', + rawCarousels: { recommended }, + carousels: { recommended: [{ title: 'Stale', href: '/stale', intro: 'Stale intro' }] }, + renderProp: vi.fn().mockResolvedValue(''), + renderTitle: vi.fn().mockResolvedValue('Landing'), + }) as unknown as Page + +const context = { + currentLanguage: 'en', + currentVersion: 'enterprise-server@3.19', +} as unknown as Context + +describe('landing page carousels', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + const transformers = [ + ['discovery-landing', new DiscoveryLandingTransformer()], + ['bespoke-landing', new BespokeLandingTransformer()], + ] as const + + const recommendedHrefs = (data: TemplateData) => + data.sections + .find((section) => section.title === 'Recommended') + ?.groups.flatMap((group) => group.links.map((link) => link.href)) + + for (const [layout, transformer] of transformers) { + const prepareTemplateData = ( + transformer as unknown as { prepareTemplateData: PrepareTemplateData } + ).prepareTemplateData.bind(transformer) + + test(`${layout} resolves rawCarousels for the requested version`, async () => { + const data = await prepareTemplateData(makePage(layout), '/en/landing', context) + + expect(vi.mocked(getLinkData)).toHaveBeenCalledWith('/a', context, { + title: true, + intro: true, + }) + expect(recommendedHrefs(data)).toEqual(['/en/enterprise-server@3.19/a']) + }) + + test(`${layout} falls back to paths relative to the landing page`, async () => { + const page = makePage(layout, ['/a', '/b', '/missing']) + const data = await prepareTemplateData(page, '/en/landing', context) + + expect(recommendedHrefs(data)).toEqual([ + '/en/enterprise-server@3.19/a', + '/en/enterprise-server@3.19/landing/b', + ]) + }) + + test(`${layout} skips pages unavailable in the requested version`, async () => { + const page = makePage(layout, ['/cloud-only', '/a']) + const data = await prepareTemplateData(page, '/en/landing', context) + + expect(recommendedHrefs(data)).toEqual(['/en/enterprise-server@3.19/a']) + }) + } +}) diff --git a/src/article-api/tests/resolve-path.ts b/src/article-api/tests/resolve-path.ts index 3b2b9560ff0e..f9c24ebe306f 100644 --- a/src/article-api/tests/resolve-path.ts +++ b/src/article-api/tests/resolve-path.ts @@ -107,6 +107,64 @@ describe('resolvePath', () => { }) }) + describe('content-root absolute paths', () => { + test('resolves /content/ href to the language-prefixed page', () => { + const page = createMockPage('pull-requests/x.md') + const context = createContext({ + '/en/pull-requests/x': page, + }) + const result = resolvePath('/content/pull-requests/x', 'en', '/en/copilot', context) + expect(result).toBe(page) + }) + + test('resolves /content/ href through a versioned key', () => { + const page = createMockPage('admin/x.md') + const context = createContext({ + '/en/enterprise-cloud@latest/admin/x': page, + }) + const result = resolvePath('/content/admin/x/', 'en', '/en/copilot', context) + expect(result).toBe(page) + }) + + test('resolves /content/ href for a non-English language', () => { + const enPage = createMockPage('pull-requests/x.md') + const jaPage = createMockPage('pull-requests/x.md') + const context = createContext({ + '/en/pull-requests/x': enPage, + '/ja/pull-requests/x': jaPage, + }) + const result = resolvePath('/content/pull-requests/x', 'ja', '/ja/copilot', context) + expect(result).toBe(jaPage) + }) + + test('skips /content/ children not available in the current version', () => { + const ghesVersion = 'enterprise-server@3.20' + const dotcomOnly = { + ...createMockPage('pull-requests/dotcom-only.md'), + applicableVersions: ['free-pro-team@latest', 'enterprise-cloud@latest'], + } as unknown as Page + const everywhere = { + ...createMockPage('rest/everywhere.md'), + applicableVersions: ['free-pro-team@latest', 'enterprise-cloud@latest', ghesVersion], + } as unknown as Page + const pages = { + '/en/pull-requests/dotcom-only': dotcomOnly, + '/en/rest/everywhere': everywhere, + } + const ghesContext = { pages, currentVersion: ghesVersion } as unknown as Context + const dotcomContext = { pages, currentVersion: 'free-pro-team@latest' } as unknown as Context + const pathname = `/en/${ghesVersion}/pull-requests` + + expect( + resolvePath('/content/pull-requests/dotcom-only', 'en', pathname, ghesContext), + ).toBeUndefined() + expect(resolvePath('/content/rest/everywhere', 'en', pathname, ghesContext)).toBe(everywhere) + expect( + resolvePath('/content/pull-requests/dotcom-only', 'en', '/en/pull-requests', dotcomContext), + ).toBe(dotcomOnly) + }) + }) + describe('not found cases', () => { test('returns undefined when page does not exist', () => { const context = createContext({ diff --git a/src/article-api/tests/rest-transformer.test.ts b/src/article-api/tests/rest-transformer.test.ts new file mode 100644 index 000000000000..f5ff659dc5c8 --- /dev/null +++ b/src/article-api/tests/rest-transformer.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, test, vi } from 'vitest' + +import { RestTransformer } from '@/article-api/transformers/rest-transformer' +import getRest from '@/rest/lib/index' +import { allVersions } from '@/versions/lib/all-versions' +import type { CodeSample, Operation } from '@/rest/components/types' +import type { Context, Page } from '@/types' + +vi.mock('@/rest/lib/index', () => ({ default: vi.fn() })) + +const stubPage = { + autogenerated: 'rest', + relativePath: 'rest/example/example.md', + title: 'Example', + intro: '', + markdown: '', + renderProp: vi.fn().mockResolvedValue(''), +} as unknown as Page + +const context = { + currentVersion: 'free-pro-team@latest', + currentVersionObj: allVersions['free-pro-team@latest'], + allVersions, +} as unknown as Context + +const makeExample = (contentType: string, example: Record = {}): CodeSample => + ({ + request: { + description: `Example ${contentType}`, + acceptHeader: 'application/vnd.github.v3+json', + parameters: { ref: 'REF' }, + }, + response: { statusCode: '200', contentType, example }, + }) as unknown as CodeSample + +const makeOperation = (overrides: Partial): Operation => + ({ + serverUrl: 'https://api.github.com', + verb: 'get', + title: 'Example', + descriptionHTML: '', + category: 'example', + subcategory: 'example', + statusCodes: [], + bodyParameters: [], + parameters: [], + codeExamples: [], + ...overrides, + }) as Operation + +const queryParam = (name: string) => ({ + name, + in: 'query', + required: true, + description: '', + schema: { type: 'string' }, +}) + +async function render(operations: Operation[]) { + vi.mocked(getRest).mockResolvedValue({ example: operations } as never) + return new RestTransformer().transform(stubPage, '/en/rest/example/example', context) +} + +describe('RestTransformer code examples', () => { + test('uses the response media type as the Accept header', async () => { + const body = await render([ + makeOperation({ + title: 'Get a commit', + requestPath: '/repos/{ref}/commits', + codeExamples: [ + makeExample('application/json'), + makeExample('application/vnd.github.diff'), + makeExample('application/vnd.github.sha'), + ], + }), + ]) + + expect(body).toContain('-H "Accept: application/vnd.github.diff"') + expect(body).toContain('-H "Accept: application/vnd.github.sha"') + expect(body).not.toContain('-H "Accept: application/json"') + expect(body).not.toContain('-H "Accept: application/vnd.github+json"') + }) + + test('appends required query parameters and quotes the URL', async () => { + const body = await render([ + makeOperation({ + title: 'Search code', + requestPath: '/search/code', + parameters: [queryParam('q')], + codeExamples: [makeExample('application/json')], + }), + makeOperation({ + title: 'Search labels', + requestPath: '/search/labels', + parameters: [queryParam('repository_id'), queryParam('q')], + codeExamples: [makeExample('application/json', { repository_id: '42' })], + }), + ]) + + expect(body).toContain('"https://api.github.com/search/code?q=Q"') + expect(body).toContain('"https://api.github.com/search/labels?repository_id=42&q=Q"') + }) + + test('leaves URLs without a query string unquoted', async () => { + const body = await render([ + makeOperation({ + requestPath: '/repos/{ref}/commits', + codeExamples: [makeExample('application/json')], + }), + ]) + + expect(body).toContain(' https://api.github.com/repos/REF/commits\n') + }) +}) diff --git a/src/article-api/transformers/bespoke-landing-transformer.ts b/src/article-api/transformers/bespoke-landing-transformer.ts index 747a275079fb..2d95160ba0bf 100644 --- a/src/article-api/transformers/bespoke-landing-transformer.ts +++ b/src/article-api/transformers/bespoke-landing-transformer.ts @@ -1,13 +1,13 @@ -import type { Context, Page, ResolvedArticle } from '@/types' -import type { PageTransformer, TemplateData, Section, LinkData } from './types' +import type { Context, Page } from '@/types' +import type { PageTransformer, TemplateData, Section } from './types' import { renderContent } from '@/content-render/index' import { loadTemplate } from '@/article-api/lib/load-template' import { getAllTocItems, flattenTocItems } from '@/article-api/lib/get-all-toc-items' +import { resolveCarouselLinks } from '@/article-api/lib/resolve-carousel-links' interface BespokeLandingPage extends Omit { featuredLinks?: Record> children?: string[] - carousels?: Record rawCarousels?: Record includedCategories?: string[] } @@ -42,34 +42,13 @@ export class BespokeLandingTransformer implements PageTransformer { const bespokePage = page as BespokeLandingPage const sections: Section[] = [] - const carousels = bespokePage.carousels ?? bespokePage.rawCarousels + // resolve-carousels overwrites page.carousels on the shared Page with another version's links. + const carousels = bespokePage.rawCarousels if (carousels && typeof carousels === 'object') { - const { default: getPageLinkData } = await import('@/frame/lib/get-link-data') - for (const [carouselKey, articles] of Object.entries(carousels)) { if (!Array.isArray(articles) || articles.length === 0) continue - let links: LinkData[] - if (typeof articles[0] === 'object' && 'title' in articles[0]) { - links = articles.map((item) => ({ - href: typeof item === 'string' ? item : item.href, - title: (typeof item === 'object' && item.title) || '', - intro: (typeof item === 'object' && item.intro) || '', - })) - } else { - const linkData = await getPageLinkData(articles as string[], context, { - title: true, - intro: true, - }) - links = (linkData || []).map( - (item: { href: string; title?: string; intro?: string }) => ({ - href: item.href, - title: item.title || '', - intro: item.intro || '', - }), - ) - } - + const links = await resolveCarouselLinks(articles, page, context) const validLinks = links.filter((l) => l.href && l.title) if (validLinks.length > 0) { const sectionTitle = carouselKey.charAt(0).toUpperCase() + carouselKey.slice(1) diff --git a/src/article-api/transformers/category-landing-transformer.ts b/src/article-api/transformers/category-landing-transformer.ts index d267baf94c69..67534880f92b 100644 --- a/src/article-api/transformers/category-landing-transformer.ts +++ b/src/article-api/transformers/category-landing-transformer.ts @@ -49,7 +49,7 @@ export class CategoryLandingTransformer implements PageTransformer { if (!parentPage.relativePath.endsWith('index.md')) { const linkData = await getLinkData(href, languageCode, pathname, context, resolvePath) - if (linkData.href) { + if (linkData) { allArticles.push(linkData) } } @@ -88,7 +88,7 @@ export class CategoryLandingTransformer implements PageTransformer { const spotlight = categoryPage.spotlight if (spotlight && spotlight.length > 0) { const links = await Promise.all( - spotlight.map(async (item) => { + spotlight.map(async (item): Promise => { const linkData = await getLinkData( item.article, languageCode, @@ -96,6 +96,7 @@ export class CategoryLandingTransformer implements PageTransformer { context, resolvePath, ) + if (!linkData) return null return { ...linkData, intro: linkData.intro @@ -105,7 +106,7 @@ export class CategoryLandingTransformer implements PageTransformer { }), ) - const validLinks = links.filter((l) => l.href) + const validLinks = links.filter((l): l is LinkData => l !== null) if (validLinks.length > 0) { sections.push({ title: 'Spotlight', diff --git a/src/article-api/transformers/discovery-landing-transformer.ts b/src/article-api/transformers/discovery-landing-transformer.ts index babbbf9f5021..331b6f46e006 100644 --- a/src/article-api/transformers/discovery-landing-transformer.ts +++ b/src/article-api/transformers/discovery-landing-transformer.ts @@ -1,13 +1,13 @@ -import type { Context, Page, ResolvedArticle } from '@/types' +import type { Context, Page } from '@/types' import type { PageTransformer, TemplateData, Section, LinkData } from './types' import { renderContent } from '@/content-render/index' import { loadTemplate } from '@/article-api/lib/load-template' import { getAllTocItems, flattenTocItems } from '@/article-api/lib/get-all-toc-items' +import { resolveCarouselLinks } from '@/article-api/lib/resolve-carousel-links' interface DiscoveryPage extends Page { rawIntroLinks?: Record introLinks?: Record - carousels?: Record rawCarousels?: Record includedCategories?: string[] children?: string[] @@ -42,34 +42,13 @@ export class DiscoveryLandingTransformer implements PageTransformer { const discoveryPage = page as DiscoveryPage const sections: Section[] = [] - const carousels = discoveryPage.carousels ?? discoveryPage.rawCarousels + // resolve-carousels overwrites page.carousels on the shared Page with another version's links. + const carousels = discoveryPage.rawCarousels if (carousels && typeof carousels === 'object') { - const { default: getPageLinkData } = await import('@/frame/lib/get-link-data') - for (const [carouselKey, articles] of Object.entries(carousels)) { if (!Array.isArray(articles) || articles.length === 0) continue - let links: LinkData[] - if (typeof articles[0] === 'object' && 'title' in articles[0]) { - links = articles.map((item) => ({ - href: typeof item === 'string' ? item : item.href, - title: (typeof item === 'object' && item.title) || '', - intro: (typeof item === 'object' && item.intro) || '', - })) - } else { - const linkData = await getPageLinkData(articles as string[], context, { - title: true, - intro: true, - }) - links = (linkData || []).map( - (item: { href: string; title?: string; intro?: string }) => ({ - href: item.href, - title: item.title || '', - intro: item.intro || '', - }), - ) - } - + const links = await resolveCarouselLinks(articles, page, context) const validLinks = links.filter((l) => l.href && l.title) if (validLinks.length > 0) { const sectionTitle = carouselKey.charAt(0).toUpperCase() + carouselKey.slice(1) diff --git a/src/article-api/transformers/journey-landing-transformer.ts b/src/article-api/transformers/journey-landing-transformer.ts index b0440765e158..302b25918759 100644 --- a/src/article-api/transformers/journey-landing-transformer.ts +++ b/src/article-api/transformers/journey-landing-transformer.ts @@ -68,7 +68,7 @@ export class JourneyLandingTransformer implements PageTransformer { return linkData }), ) - const validLinks = links.filter((l): l is LinkData => l !== null && !!l.href) + const validLinks = links.filter((l): l is LinkData => l !== null) if (validLinks.length > 0) { groups.push({ title: track.title, links: validLinks }) } @@ -88,7 +88,7 @@ export class JourneyLandingTransformer implements PageTransformer { return await getLinkData(childHref, languageCode, pathname, context, resolvePath) }), ) - const validLinks = links.filter((l): l is LinkData => !!l.href) + const validLinks = links.filter((l): l is LinkData => l !== null) if (validLinks.length > 0) { sections.push({ title: 'Links', diff --git a/src/article-api/transformers/rest-transformer.ts b/src/article-api/transformers/rest-transformer.ts index 667ff44d301a..8f0a22315896 100644 --- a/src/article-api/transformers/rest-transformer.ts +++ b/src/article-api/transformers/rest-transformer.ts @@ -10,6 +10,10 @@ import { getCurlBodyArguments, needsExplicitContentTypeHeader, } from '@/rest/lib/curl-body-arguments' +import { + getAcceptHeader, + getRequiredQueryParamsPath, +} from '@/rest/components/get-rest-code-samples' import matter from '@gr2m/gray-matter' import { fastTextOnly } from '@/content-render/unified/text-only' import GithubSlugger from 'github-slugger' @@ -221,6 +225,12 @@ export class RestTransformer implements PageTransformer { } } + const requiredQueryParams = getRequiredQueryParamsPath(operation, example) + if (requiredQueryParams) { + // Quote the URL so shells don't treat ? or & specially. + url = `"${url}?${requiredQueryParams}"` + } + const responseSchema = ( example.response as { schema?: Parameters[0] } | undefined )?.schema @@ -230,7 +240,7 @@ export class RestTransformer implements PageTransformer { ? fastTextOnly(example.request.description) : '', url, - acceptHeader: example.request?.acceptHeader, + acceptHeader: getAcceptHeader(example), contentType: example.request?.contentType, needsContentTypeHeader: needsExplicitContentTypeHeader(example.request?.contentType), bodyParameters: getCurlBodyArguments( diff --git a/src/article-api/transformers/toc-transformer.ts b/src/article-api/transformers/toc-transformer.ts index a53a866d7c6b..bf06eecd6364 100644 --- a/src/article-api/transformers/toc-transformer.ts +++ b/src/article-api/transformers/toc-transformer.ts @@ -1,5 +1,5 @@ import type { Context, Page } from '@/types' -import type { PageTransformer, TemplateData, Section } from './types' +import type { PageTransformer, TemplateData, Section, LinkData } from './types' import { renderContent } from '@/content-render/index' import { loadTemplate } from '@/article-api/lib/load-template' import { resolvePath } from '@/article-api/lib/resolve-path' @@ -51,7 +51,7 @@ export class TocTransformer implements PageTransformer { }), ) - const validLinks = links.filter((l) => l.href) + const validLinks = links.filter((l): l is LinkData => l !== null) if (validLinks.length > 0) { sections.push({ title: 'Links', diff --git a/src/fixtures/PLAYWRIGHT.md b/src/fixtures/PLAYWRIGHT.md index b3b5f92b6da7..b8ad5f521ddb 100644 --- a/src/fixtures/PLAYWRIGHT.md +++ b/src/fixtures/PLAYWRIGHT.md @@ -28,6 +28,14 @@ That command will automatically start a server (on `localhost:4000`) for the duration of test suite. It then finds all `tests/**/*.spec.ts` files and run them by using `Google Chrome` as the underlying browser. +The suites are `playwright-rendering`, `playwright-header`, `playwright-a11y`, +and `playwright-secret-scanning`. CI runs each one as a separate check. For +example, to run the site header tests: + +```shell +npm run playwright-test -- playwright-header +``` + If you have [set up a local Elasticsearch server](../search/elasticsearch-locally.md) (`localhost:9200`) the headless tests will test doing site-searches if you've set up an `ELASTICSEARCH_URL` environment variable. diff --git a/src/fixtures/fixtures/content/get-started/liquid/ifversion.md b/src/fixtures/fixtures/content/get-started/liquid/ifversion.md index 23f85b63ecf0..a78ccf32f9e8 100644 --- a/src/fixtures/fixtures/content/get-started/liquid/ifversion.md +++ b/src/fixtures/fixtures/content/get-started/liquid/ifversion.md @@ -1,6 +1,6 @@ --- title: Ifversion liquid tag -intro: 'Tests functionality of `ifversion` and its operators {% ifversion not fpt %}(not on fpt){% else %}(on fpt){% endif %}' +intro: 'Tests functionality of `ifversion` and its operators {% ifversion not fpt %}(not on fpt){% else %}(on fpt){% endif %} {% ifversion ghes %}(bare ghes){% else %}(bare not ghes){% endif %}' versions: fpt: '*' ghes: '*' diff --git a/src/fixtures/fixtures/content/rest/about-the-rest-api/comparing-githubs-rest-api-and-graphql-api.md b/src/fixtures/fixtures/content/rest/about-the-rest-api/comparing-githubs-rest-api-and-graphql-api.md index 5efb318379c6..418a684f9653 100644 --- a/src/fixtures/fixtures/content/rest/about-the-rest-api/comparing-githubs-rest-api-and-graphql-api.md +++ b/src/fixtures/fixtures/content/rest/about-the-rest-api/comparing-githubs-rest-api-and-graphql-api.md @@ -10,3 +10,7 @@ versions: ## About {% data variables.product.company_short %}'s APIs First paragraph right here. + +{% if query.apiVersion == nil or "2026-03-10" <= query.apiVersion %} +This paragraph applies to API version 2026-03-10 and later. +{% endif %} diff --git a/src/fixtures/tests/playwright-header.spec.ts b/src/fixtures/tests/playwright-header.spec.ts index 8fdeb5661b79..83cef866c211 100644 --- a/src/fixtures/tests/playwright-header.spec.ts +++ b/src/fixtures/tests/playwright-header.spec.ts @@ -1175,6 +1175,8 @@ test.describe('Brand header', () => { }) => { await page.setViewportSize({ width: 390, height: 800 }) await page.goto(ARTICLE) + const body = page.locator('body') + const originalOverflow = await body.evaluate((element) => element.style.overflow) await page.getByRole('button', { name: 'Menu', exact: true }).click() await expectBackgroundIsolated(page, true) @@ -1191,10 +1193,13 @@ test.describe('Brand header', () => { 'false', ) await expectBackgroundIsolated(page, false) - await expect(page.locator('body')).not.toHaveCSS('overflow', 'hidden') + // Docs search locks body scroll while open, so the menu's release shows only after it closes. await page.keyboard.press('Escape') await expect(searchTrigger).toBeFocused() + await expect + .poll(() => body.evaluate((element) => element.style.overflow)) + .toBe(originalOverflow) }) } diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index dbbc831cd3ff..3f091c516e1b 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -1818,6 +1818,18 @@ test.describe('rest API reference pages', () => { await expect(page).toHaveURL(/\/en\/rest\/actions\/artifacts\?apiVersion=/) await expect(page).toHaveTitle(/GitHub Actions Artifacts - GitHub Docs/) }) + + test('remember selected API version', async ({ page }) => { + await page.goto('/rest/actions/artifacts') + await expect(page).toHaveURL(/\/en\/rest\/actions\/artifacts\?apiVersion=/) + await page.getByRole('button', { name: 'Select API Version' }).click() + await page.getByRole('menuitemradio', { name: '2022-11-28', exact: true }).click() + await expect(page).toHaveURL(/\/en\/rest\/actions\/artifacts\?apiVersion=2022-11-28/) + + // Without an apiVersion query, RestRedirect uses the saved cookie instead of the latest version. + await page.goto('/rest/actions/cache') + await expect(page).toHaveURL(/\/en\/rest\/actions\/cache\?apiVersion=2022-11-28/) + }) }) test.describe('translations', () => { diff --git a/src/frame/middleware/block-robots.ts b/src/frame/middleware/block-robots.ts index c49e8f602f98..82bd0e604065 100644 --- a/src/frame/middleware/block-robots.ts +++ b/src/frame/middleware/block-robots.ts @@ -1,15 +1,24 @@ import type { Request, Response, NextFunction } from 'express' +import { escapeRegExp } from 'lodash-es' import { productMap } from '@/products/lib/all-products' import { deprecated } from '@/versions/lib/enterprise-server-releases' const pathRegExps: RegExp[] = [ // WIP and hidden products stay out of search indexes. + // Match whole segments so a hidden /search does not block /search-github or /rest/search. + // A trailing .md matches too because Markdown responses have no robots meta tag. ...Object.values(productMap) .filter((product) => product.wip || product.hidden) .map((product) => [ - new RegExp(`^/.*?${product.href}`, 'i'), - ...product.versions!.map((version) => new RegExp(`^/.*?${version}/${product.id}`, 'i')), + new RegExp(`^/[^/]+${escapeRegExp(product.href)}(/|\\.md$|$)`, 'i'), + ...product.versions!.map( + (version) => + new RegExp( + `^/[^/]+/${escapeRegExp(version)}/${escapeRegExp(product.id)}(/|\\.md$|$)`, + 'i', + ), + ), ]), // Deprecated enterprise versions stay out of search indexes. diff --git a/src/frame/tests/block-robots.ts b/src/frame/tests/block-robots.ts index 1c642ddc156b..dee48be78290 100644 --- a/src/frame/tests/block-robots.ts +++ b/src/frame/tests/block-robots.ts @@ -26,8 +26,7 @@ describe('block robots', () => { `/en${href}`, `/en${href}/overview`, `/en${href}/overview/intro`, - `/en/enterprise/${enterpriseServerReleases.latest}/user${href}`, - `/en/enterprise/${enterpriseServerReleases.oldestSupported}/user${href}`, + `/ja${href}`, ] for (const path of blockedPaths) { @@ -56,6 +55,33 @@ describe('block robots', () => { } }) + test('disallows crawling of the hidden search page', async () => { + expect(productMap.search.hidden).toBe(true) + expect(allowIndex('/en/search')).toBe(false) + expect(allowIndex('/ja/search')).toBe(false) + expect(allowIndex('/en/enterprise-cloud@latest/search')).toBe(false) + expect(allowIndex('/en/search.md')).toBe(false) + expect(allowIndex('/ja/search.md')).toBe(false) + expect(allowIndex('/en/enterprise-cloud@latest/search.md')).toBe(false) + }) + + test('allows crawling of public pages whose path contains "search"', async () => { + expect(productMap['search-github'].hidden).toBe(false) + const allowedPaths = [ + '/en/search-github', + '/en/search-github.md', + '/en/search-github/searching-on-github/searching-code', + '/ja/enterprise-cloud@latest/search-github/searching-on-github/searching-code', + '/en/rest/search/search', + '/en/graphql/reference/search', + '/en/repositories/releasing-projects-on-github/searching-a-repositorys-releases', + ] + + for (const path of allowedPaths) { + expect(allowIndex(path)).toBe(true) + } + }) + test('allows crawling of non-WIP products', async () => { expect('actions' in productMap).toBe(true) expect(allowIndex('/en/actions')).toBe(true) diff --git a/src/ghes-releases/lib/enterprise-dates.json b/src/ghes-releases/lib/enterprise-dates.json index 5e80070978bf..ab9332c61a2d 100644 --- a/src/ghes-releases/lib/enterprise-dates.json +++ b/src/ghes-releases/lib/enterprise-dates.json @@ -253,7 +253,7 @@ }, "3.18": { "releaseDate": "2025-09-03", - "deprecationDate": "2026-10-14", + "deprecationDate": "2026-10-16", "releaseCandidateDate": "2025-09-03", "generalAvailabilityDate": "2025-10-14" }, diff --git a/src/github-apps/scripts/sync.ts b/src/github-apps/scripts/sync.ts index 48b3d4e66a94..49ceaed09c04 100755 --- a/src/github-apps/scripts/sync.ts +++ b/src/github-apps/scripts/sync.ts @@ -162,21 +162,14 @@ export async function syncGitHubAppsData( addAppData(githubAppsData['user-to-server-rest'], category, appDataOperation) } - if (isFineGrainedPat) { - const allPermissionSetsExcluded = progAccessData[operation.operationId].permissions.every( - (permissionSet) => - Object.keys(permissionSet).every((permissionName) => - isActorExcluded( - progActorResources[permissionName]?.excluded_actors, - 'fine_grained_pat', - actorTypeMap, - ), - ), + if ( + isFineGrainedPat && + !isOperationExcludedForFineGrainedPat( + progAccessData[operation.operationId].permissions, + progActorResources, ) - - if (!allPermissionSetsExcluded) { - addAppData(githubAppsData['fine-grained-pat'], category, appDataOperation) - } + ) { + addAppData(githubAppsData['fine-grained-pat'], category, appDataOperation) } for (const permissionSet of progAccessData[operation.operationId].permissions) { @@ -568,6 +561,26 @@ export function isActorExcluded( return false } + +// Operations with no permission sets need no permissions, so they still work with fine-grained PATs. +export function isOperationExcludedForFineGrainedPat( + permissionSets: Array>, + progActorResources: ProgActorResources, +): boolean { + return ( + permissionSets.length > 0 && + permissionSets.every((permissionSet) => + Object.keys(permissionSet).every((permissionName) => + isActorExcluded( + progActorResources[permissionName]?.excluded_actors, + 'fine_grained_pat', + actorTypeMap, + ), + ), + ) + ) +} + function addAppData( storage: Record, category: string, diff --git a/src/github-apps/tests/excluded-actors.ts b/src/github-apps/tests/excluded-actors.ts index 3041a1652146..23d5a5c48aca 100644 --- a/src/github-apps/tests/excluded-actors.ts +++ b/src/github-apps/tests/excluded-actors.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from 'vitest' -import { isActorExcluded, actorTypeMap } from '../scripts/sync' +import { + isActorExcluded, + actorTypeMap, + isOperationExcludedForFineGrainedPat, +} from '../scripts/sync' describe('excluded_actors filtering', () => { test('returns false when excludedActors is undefined', () => { @@ -113,3 +117,35 @@ describe('excluded_actors filtering', () => { expect(isActorExcluded(bothValues, 'fine_grained_pat', actorTypeMap)).toBe(true) }) }) + +describe('isOperationExcludedForFineGrainedPat', () => { + const progActorResources = { + checks: { excluded_actors: ['fine_grained_personal_access_token'] }, + contents: {}, + } + + test('keeps operations that have no permission sets', () => { + expect(isOperationExcludedForFineGrainedPat([], progActorResources)).toBe(false) + }) + + test('excludes operations when every permission set excludes fine-grained PATs', () => { + expect(isOperationExcludedForFineGrainedPat([{ checks: 'read' }], progActorResources)).toBe( + true, + ) + }) + + test('keeps operations when any permission set allows fine-grained PATs', () => { + expect( + isOperationExcludedForFineGrainedPat( + [{ checks: 'read' }, { contents: 'read' }], + progActorResources, + ), + ).toBe(false) + }) + + test('keeps operations whose permissions have no excluded actors', () => { + expect(isOperationExcludedForFineGrainedPat([{ contents: 'read' }], progActorResources)).toBe( + false, + ) + }) +}) diff --git a/src/graphql/data/fpt/schema-pulls.json b/src/graphql/data/fpt/schema-pulls.json index 64e4afa795b1..fc87b6686822 100644 --- a/src/graphql/data/fpt/schema-pulls.json +++ b/src/graphql/data/fpt/schema-pulls.json @@ -5252,7 +5252,7 @@ }, { "name": "requiredReviewers", - "description": "

This field is in beta and subject to change. A collection of reviewers and\nassociated file patterns. Each reviewer has a list of file patterns which\ndetermine the files that reviewer is required to review.

", + "description": "

A collection of reviewers and associated file patterns. Each reviewer has a\nlist of file patterns which determine the files that reviewer is required to review.

", "type": "[RequiredReviewerConfiguration!]", "id": "requiredreviewerconfiguration", "href": "/graphql/reference/pulls#object-requiredreviewerconfiguration" @@ -10559,7 +10559,7 @@ }, { "name": "requiredReviewers", - "description": "

This argument is in beta and subject to change. A collection of reviewers and\nassociated file patterns. Each reviewer has a list of file patterns which\ndetermine the files that reviewer is required to review.

", + "description": "

A collection of reviewers and associated file patterns. Each reviewer has a\nlist of file patterns which determine the files that reviewer is required to review.

", "type": "[RequiredReviewerConfigurationInput!]", "id": "requiredreviewerconfigurationinput", "href": "/graphql/reference/pulls#input-object-requiredreviewerconfigurationinput" diff --git a/src/graphql/data/fpt/schema.docs.graphql b/src/graphql/data/fpt/schema.docs.graphql index 7fd1c0a2d5b3..8b827172b1ed 100644 --- a/src/graphql/data/fpt/schema.docs.graphql +++ b/src/graphql/data/fpt/schema.docs.graphql @@ -46069,9 +46069,8 @@ type PullRequestParameters @docsCategory(name: "pulls") { requiredReviewThreadResolution: Boolean! """ - This field is in beta and subject to change. A collection of reviewers and - associated file patterns. Each reviewer has a list of file patterns which - determine the files that reviewer is required to review. + A collection of reviewers and associated file patterns. Each reviewer has a + list of file patterns which determine the files that reviewer is required to review. """ requiredReviewers: [RequiredReviewerConfiguration!] } @@ -46117,9 +46116,8 @@ input PullRequestParametersInput @docsCategory(name: "pulls") { requiredReviewThreadResolution: Boolean! """ - This argument is in beta and subject to change. A collection of reviewers and - associated file patterns. Each reviewer has a list of file patterns which - determine the files that reviewer is required to review. + A collection of reviewers and associated file patterns. Each reviewer has a + list of file patterns which determine the files that reviewer is required to review. """ requiredReviewers: [RequiredReviewerConfigurationInput!] } diff --git a/src/graphql/data/ghec/schema-pulls.json b/src/graphql/data/ghec/schema-pulls.json index 8351bcee7b27..50a09d16e35e 100644 --- a/src/graphql/data/ghec/schema-pulls.json +++ b/src/graphql/data/ghec/schema-pulls.json @@ -5252,7 +5252,7 @@ }, { "name": "requiredReviewers", - "description": "

This field is in beta and subject to change. A collection of reviewers and\nassociated file patterns. Each reviewer has a list of file patterns which\ndetermine the files that reviewer is required to review.

", + "description": "

A collection of reviewers and associated file patterns. Each reviewer has a\nlist of file patterns which determine the files that reviewer is required to review.

", "type": "[RequiredReviewerConfiguration!]", "id": "requiredreviewerconfiguration", "href": "/graphql/reference/pulls#object-requiredreviewerconfiguration" @@ -10559,7 +10559,7 @@ }, { "name": "requiredReviewers", - "description": "

This argument is in beta and subject to change. A collection of reviewers and\nassociated file patterns. Each reviewer has a list of file patterns which\ndetermine the files that reviewer is required to review.

", + "description": "

A collection of reviewers and associated file patterns. Each reviewer has a\nlist of file patterns which determine the files that reviewer is required to review.

", "type": "[RequiredReviewerConfigurationInput!]", "id": "requiredreviewerconfigurationinput", "href": "/graphql/reference/pulls#input-object-requiredreviewerconfigurationinput" diff --git a/src/graphql/data/ghec/schema.docs.graphql b/src/graphql/data/ghec/schema.docs.graphql index 7fd1c0a2d5b3..8b827172b1ed 100644 --- a/src/graphql/data/ghec/schema.docs.graphql +++ b/src/graphql/data/ghec/schema.docs.graphql @@ -46069,9 +46069,8 @@ type PullRequestParameters @docsCategory(name: "pulls") { requiredReviewThreadResolution: Boolean! """ - This field is in beta and subject to change. A collection of reviewers and - associated file patterns. Each reviewer has a list of file patterns which - determine the files that reviewer is required to review. + A collection of reviewers and associated file patterns. Each reviewer has a + list of file patterns which determine the files that reviewer is required to review. """ requiredReviewers: [RequiredReviewerConfiguration!] } @@ -46117,9 +46116,8 @@ input PullRequestParametersInput @docsCategory(name: "pulls") { requiredReviewThreadResolution: Boolean! """ - This argument is in beta and subject to change. A collection of reviewers and - associated file patterns. Each reviewer has a list of file patterns which - determine the files that reviewer is required to review. + A collection of reviewers and associated file patterns. Each reviewer has a + list of file patterns which determine the files that reviewer is required to review. """ requiredReviewers: [RequiredReviewerConfigurationInput!] } diff --git a/src/rest/components/ApiVersionPicker.tsx b/src/rest/components/ApiVersionPicker.tsx index fb9feaa21bd1..47b0465eaa86 100644 --- a/src/rest/components/ApiVersionPicker.tsx +++ b/src/rest/components/ApiVersionPicker.tsx @@ -55,7 +55,6 @@ export const ApiVersionPicker = () => { href: itemLink, extra: { info: false, - currentDate, }, } }) @@ -68,7 +67,6 @@ export const ApiVersionPicker = () => { }${apiVersionPath}`, extra: { info: true, - currentDate, }, }) @@ -84,7 +82,7 @@ export const ApiVersionPicker = () => { dataTestId="version" ariaLabel="Select API Version" onSelect={(item) => { - if (item.extra?.currentDate) rememberApiVersion(item.extra.currentDate) + if (!item.extra?.info) rememberApiVersion(item.text) }} renderItem={(item) => { return item.extra?.info ? ( diff --git a/src/rest/components/get-rest-code-samples.ts b/src/rest/components/get-rest-code-samples.ts index a3a017ab3c30..992a170b1fe1 100644 --- a/src/rest/components/get-rest-code-samples.ts +++ b/src/rest/components/get-rest-code-samples.ts @@ -375,7 +375,7 @@ function findMatchingQueryKey(exampleObj: CodeExamples | CodeExamples[], matchKe return match } -function getRequiredQueryParamsPath(operation: Operation, codeSample: CodeSample) { +export function getRequiredQueryParamsPath(operation: Operation, codeSample: CodeSample) { const requiredQueryParams = new URLSearchParams() for (const param of operation.parameters) { if (param.in === 'query' && param.required === true) { @@ -388,7 +388,7 @@ function getRequiredQueryParamsPath(operation: Operation, codeSample: CodeSample return requiredQueryParams.toString() } -function getAcceptHeader(codeSample: CodeSample) { +export function getAcceptHeader(codeSample: CodeSample) { const contentType = codeSample?.response?.contentType if (!contentType || contentType === 'application/json') { diff --git a/src/rest/scripts/utils/get-body-params.ts b/src/rest/scripts/utils/get-body-params.ts index d275af5b7f53..aa96b09c9e3e 100644 --- a/src/rest/scripts/utils/get-body-params.ts +++ b/src/rest/scripts/utils/get-body-params.ts @@ -37,6 +37,11 @@ interface BodyParamProps { // and Set status check contexts operations need this. // When every top-level oneOf option is an object, getTopLevelOneOfProperty merges // all properties. With three or more options, middle required fields can lose required flags. +// When the parent schema has properties, getTopLevelOneOfProperty keeps them and the parent +// required list, because options like those in Create a check run only constrain parent fields. +// Options add missing properties: every option when all are objects, otherwise the first +// option. Option required fields apply to option-added properties, but a parent property +// becomes required only when every option requires it. async function getTopLevelOneOfProperty( schema: Schema, ): Promise<{ properties: Record; required: string[] }> { @@ -47,9 +52,29 @@ async function getTopLevelOneOfProperty( throw new Error('Schema requestBody oneOf property is not an array') } + const allOneOfAreObjects = schema.oneOf.every((elem) => elem.type === 'object') + + if (schema.properties) { + const parentProperties = schema.properties + const properties = { ...parentProperties } + const required = new Set(schema.required || []) + const mergedOptions = allOneOfAreObjects ? schema.oneOf : [schema.oneOf[0]] + for (const each of mergedOptions) { + for (const [key, value] of Object.entries(each.properties || {})) { + if (!Object.hasOwn(properties, key)) properties[key] = value + } + for (const name of each.required || []) { + const requiredByEveryOption = schema.oneOf.every((option) => + option.required?.includes(name), + ) + if (requiredByEveryOption || !Object.hasOwn(parentProperties, name)) required.add(name) + } + } + return { properties, required: [...required] } + } + // When oneOf types differ, the first option must be the comprehensive object form. const firstOneOfObject = schema.oneOf[0] - const allOneOfAreObjects = schema.oneOf.every((elem) => elem.type === 'object') let required = firstOneOfObject.required || [] let properties = firstOneOfObject.properties || {} diff --git a/src/rest/scripts/utils/tests/get-body-params.test.ts b/src/rest/scripts/utils/tests/get-body-params.test.ts index ba2e1aecba23..7210ea7780e7 100644 --- a/src/rest/scripts/utils/tests/get-body-params.test.ts +++ b/src/rest/scripts/utils/tests/get-body-params.test.ts @@ -104,6 +104,139 @@ describe('getBodyParams — OAS 3.1 nullable handling', () => { ]) }) + it('keeps parent properties when top-level oneOf options only constrain them', async () => { + const schema: Schema = { + type: 'object', + properties: { + name: { type: 'string', description: 'The name' }, + head_sha: { type: 'string', description: 'The SHA' }, + status: { + type: 'string', + description: 'The status', + enum: ['queued', 'in_progress', 'completed'], + }, + conclusion: { type: 'string', description: 'The conclusion' }, + output: { type: 'object', description: 'The output' }, + }, + required: ['name', 'head_sha'], + oneOf: [ + { + properties: { status: { enum: ['completed'] } }, + required: ['status', 'conclusion'], + }, + { + properties: { status: { enum: ['queued', 'in_progress'] } }, + }, + ], + } + const params = await getBodyParams(schema, true) + expect(params.map(({ name, type, isRequired }) => [name, type, isRequired])).toEqual([ + ['name', 'string', true], + ['head_sha', 'string', true], + ['status', 'string', undefined], + ['conclusion', 'string', undefined], + ['output', 'object', undefined], + ]) + expect(params[2].enum).toEqual(['queued', 'in_progress', 'completed']) + }) + + it('keeps parent properties when top-level oneOf options only list required fields', async () => { + const schema: Schema = { + type: 'object', + properties: { + type: { type: 'string', description: 'The type', enum: ['Issue', 'PullRequest'] }, + id: { type: 'integer', description: 'The ID' }, + owner: { type: 'string', description: 'The owner' }, + repo: { type: 'string', description: 'The repo' }, + number: { type: 'integer', description: 'The number' }, + }, + required: ['type'], + oneOf: [{ required: ['id'] }, { required: ['owner', 'repo', 'number'] }], + } + const params = await getBodyParams(schema, true) + expect(params.map(({ name, isRequired }) => [name, isRequired])).toEqual([ + ['type', true], + ['id', undefined], + ['owner', undefined], + ['repo', undefined], + ['number', undefined], + ]) + }) + + it('keeps alternative parent fields optional when top-level oneOf options are typed objects', async () => { + const schema: Schema = { + type: 'object', + properties: { + type: { type: 'string', description: 'The type', enum: ['Issue', 'PullRequest'] }, + id: { type: 'integer', description: 'The ID' }, + owner: { type: 'string', description: 'The owner' }, + repo: { type: 'string', description: 'The repo' }, + number: { type: 'integer', description: 'The number' }, + }, + required: ['type'], + oneOf: [ + { type: 'object', required: ['id'] }, + { type: 'object', required: ['owner', 'repo', 'number'] }, + ], + } + const params = await getBodyParams(schema, true) + expect(params.map(({ name, isRequired }) => [name, isRequired])).toEqual([ + ['type', true], + ['id', undefined], + ['owner', undefined], + ['repo', undefined], + ['number', undefined], + ]) + }) + + it('adds first-option properties to parent properties when top-level oneOf options are untyped', async () => { + const schema: Schema = { + type: 'object', + properties: { + type: { type: 'string', description: 'The type', enum: ['Issue', 'PullRequest'] }, + }, + required: ['type'], + oneOf: [ + { + properties: { + type: { enum: ['Issue'] }, + owner: { type: 'string', description: 'The owner' }, + repo: { type: 'string', description: 'The repo' }, + number: { type: 'integer', description: 'The number' }, + }, + required: ['owner', 'repo', 'number'], + }, + { properties: { id: { type: 'integer', description: 'The ID' } }, required: ['id'] }, + ], + } + const params = await getBodyParams(schema, true) + expect(params.map(({ name, isRequired }) => [name, isRequired])).toEqual([ + ['type', true], + ['owner', true], + ['repo', true], + ['number', true], + ]) + expect(params[0].enum).toEqual(['Issue', 'PullRequest']) + }) + + it('marks a parent property required when every top-level oneOf option requires it', async () => { + const schema: Schema = { + type: 'object', + properties: { + mode: { type: 'string', description: 'The mode' }, + first: { type: 'string', description: 'First value' }, + second: { type: 'string', description: 'Second value' }, + }, + oneOf: [{ required: ['mode', 'first'] }, { required: ['second', 'mode'] }], + } + const params = await getBodyParams(schema, true) + expect(params.map(({ name, isRequired }) => [name, isRequired])).toEqual([ + ['mode', true], + ['first', undefined], + ['second', undefined], + ]) + }) + it('renders type: ["string", "null"] as "string or null"', async () => { const schema = { type: 'object', diff --git a/src/tests/SUITES.md b/src/tests/SUITES.md index 56c1e7ef28c9..7434196240a6 100644 --- a/src/tests/SUITES.md +++ b/src/tests/SUITES.md @@ -52,6 +52,7 @@ Note: for build-gating suites (data-directory, content-linter, generated-referen | Suite | Covers | Prereqs | Admin-merge risk | |---|---|---|---| | playwright-rendering | Client-side JS behavior: navigation, sidebar, search, platform/tool pickers, code tabs, hovercards, responsive layouts, versioning, and Liquid features. | Built app on :4000, Elasticsearch, and Chromium downloaded from a CDN. | **Medium**: real client-side regressions, but the CDN Chromium download flakes (disabled before for that). Confirm a red isn't the browser install. | +| playwright-header | Site header behavior: layouts at each viewport, plan and language pickers, the narrow utility menu, opening Docs search and returning focus, skip links, and signup visibility. | Built app on :4000 with fixture content, and Chromium. Combined search is mocked in `beforeEach`, so the tests don't need Elasticsearch. | **Medium**: the header is on every page, so reds are usually real. Not a required check yet. Confirm a red isn't the browser install. | | playwright-a11y | Accessibility violations on a set of representative pages via axe-core, with and without experimental feature flags. | Local Next.js fixture server; Chromium. | **Low**: explicitly non-blocking; a red may be real a11y or an axe false positive. Doesn't gate PRs. | | playwright-secret-scanning | Client-side accessibility and UI behavior of the secret-scanning patterns reference page. | CDN Chromium, dev server on :4000, and ES fixtures. | **Medium**: real UI/a11y regressions, but CDN Chromium flake adds noise. | diff --git a/src/tools/components/Fields.tsx b/src/tools/components/Fields.tsx index 7e0fcf9d9593..92537fdf92ad 100644 --- a/src/tools/components/Fields.tsx +++ b/src/tools/components/Fields.tsx @@ -26,7 +26,8 @@ export const Fields = (fieldProps: { key={item.text} href={item.href} active={item.selected === true} - onSelect={() => { + // LinkItem forwards onSelect to the anchor as a text-selection event that never fires on click. + onClick={() => { if (onSelect) onSelect(item) setOpen(!open) }} diff --git a/src/tools/components/Picker.tsx b/src/tools/components/Picker.tsx index 1d4fcf7bafdf..30b0c32e1958 100644 --- a/src/tools/components/Picker.tsx +++ b/src/tools/components/Picker.tsx @@ -27,7 +27,6 @@ export interface PickerItem { arrow?: boolean info?: boolean version?: string - currentDate?: string } divider?: boolean }