From 26c2a8b653f86185c747f93b993e803b65477c5f Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Tue, 21 Jul 2026 17:35:56 +0000 Subject: [PATCH 1/3] Extend registry checks to development registry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fc23d6a-ef10-4f60-b369-2770f0af80d6 --- .github/CODEOWNERS | 1 + .github/scripts/src/ext-registry-check.js | 69 ++++++++++--------- .../scripts/test/ext-registry-check.test.js | 69 ++++++++++++++++++- .github/workflows/ext-registry-check.yml | 18 +++-- 4 files changed, 115 insertions(+), 42 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f124cbeda73..e6a628aa268 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -12,6 +12,7 @@ # ── CLI extensions registry - anyone added to this can approve extension publishing, provided they pass the `ext-registry-check` workflow. /cli/azd/extensions/registry.json @vhvb1989 @hemarina @JeffreyCA @tg-msft @RickWinter @richardpark-msft @XiaofuHuang @achauhan-scc @anchenyi @glharper @huimiu @hund030 @kingernupur @saanikaguptamicrosoft @therealjohn @trangevi @trrwilson +/cli/azd/extensions/registry.dev.json @vhvb1989 @hemarina @JeffreyCA @tg-msft @RickWinter @richardpark-msft @XiaofuHuang @achauhan-scc @anchenyi @glharper @huimiu @hund030 @kingernupur @saanikaguptamicrosoft @therealjohn @trangevi @trrwilson # ── CLI extensions (AI) ────────────────────────────────────────────────────── /cli/azd/extensions/azure.ai.agents/ @JeffreyCA @glharper @trangevi @trrwilson @therealjohn @huimiu @hund030 diff --git a/.github/scripts/src/ext-registry-check.js b/.github/scripts/src/ext-registry-check.js index 302e80a653f..80a5c58782f 100644 --- a/.github/scripts/src/ext-registry-check.js +++ b/.github/scripts/src/ext-registry-check.js @@ -16,10 +16,13 @@ module.exports.forTests = { diffRegistry, } -const REGISTRY_JSON_PATH = 'cli/azd/extensions/registry.json'; +const REGISTRY_JSON_PATHS = new Set([ + 'cli/azd/extensions/registry.json', + 'cli/azd/extensions/registry.dev.json', +]); +const DEFAULT_REGISTRY_JSON_PATH = 'cli/azd/extensions/registry.json'; // We only allow URLs that point to our GitHub releases page. -// NOTE: this script is only for production registry.json - nightlies go to a non-releases spot, etc... const ALLOWED_ARTIFACT_URL_ORIGIN = 'https://github.com'; const ALLOWED_ARTIFACT_URL_PATH_PREFIX = '/Azure/azure-dev/releases/download/'; const ALLOWED_ARTIFACT_URL_PREFIX = `${ALLOWED_ARTIFACT_URL_ORIGIN}${ALLOWED_ARTIFACT_URL_PATH_PREFIX}`; @@ -97,18 +100,24 @@ async function run({ github: octokit, context, core, coreTeam, registryBaseRef } return; } + const changedFiles = await getChangedFiles({ octokit, context }); + // Non-registry file changes require core review. - const changedFileReviewReasons = await getChangedFileReviewReasons({ - octokit, - context, - }); + const changedFileReviewReasons = diffChangedFiles(changedFiles); // Simple release-only registry changes can proceed without core review. - const registryReviewReasons = await isAllowedRegistryJsonUpdate({ - octokit, - context, - registryBaseRef: baseRef, - }); + const changedRegistryPaths = [...REGISTRY_JSON_PATHS] + .filter((registryPath) => changedFiles.some((file) => file.filename === registryPath)); + const registryReviewReasons = []; + for (const registryPath of changedRegistryPaths) { + const reasons = await isAllowedRegistryJsonUpdate({ + octokit, + context, + registryBaseRef: baseRef, + registryPath, + }); + registryReviewReasons.push(...reasons.map((reason) => `${registryPath}: ${reason}`)); + } const reviewReasons = changedFileReviewReasons.concat(registryReviewReasons); @@ -235,10 +244,15 @@ function isCreatedByCoreTeam({ context, core, coreTeam }) { /** * Checks whether the registry update is simple enough to proceed without core-team review. * - * @param {{ octokit: Octokit, context: Context, registryBaseRef?: string }} args + * @param {{ octokit: Octokit, context: Context, registryBaseRef?: string, registryPath?: string }} args * @returns {Promise} the reasons core review is needed; empty means the change is approved */ -async function isAllowedRegistryJsonUpdate({ octokit, context, registryBaseRef = 'main' }) { +async function isAllowedRegistryJsonUpdate({ + octokit, + context, + registryBaseRef = 'main', + registryPath = DEFAULT_REGISTRY_JSON_PATH, +}) { assertHasPullRequest(context); const pr = context.payload.pull_request; @@ -247,6 +261,7 @@ async function isAllowedRegistryJsonUpdate({ octokit, context, registryBaseRef = owner: context.repo.owner, repo: context.repo.repo, ref: registryBaseRef, + registryPath, }); const head = pr['head']; @@ -260,22 +275,12 @@ async function isAllowedRegistryJsonUpdate({ octokit, context, registryBaseRef = owner: head?.repo?.owner?.login ?? context.repo.owner, repo: head?.repo?.name ?? context.repo.repo, ref, + registryPath, }); return diffRegistry(mainRegistry, prRegistry); } -/** - * Checks whether the PR changed only registry.json. - * - * @param {{ octokit: Octokit, context: Context }} args - * @returns {Promise} - */ -async function getChangedFileReviewReasons({ octokit, context }) { - const changedFiles = await getChangedFiles({ octokit, context }); - return diffChangedFiles(changedFiles); -} - /** * Fetches the list of files changed by the PR. * @@ -297,7 +302,7 @@ async function getChangedFiles({ octokit, context }) { */ function diffChangedFiles(changedFiles) { const unexpectedFiles = changedFiles - .filter((file) => file.filename !== REGISTRY_JSON_PATH || file.previous_filename != null) + .filter((file) => !REGISTRY_JSON_PATHS.has(file.filename) || file.previous_filename != null) .map((file) => file.filename); if (unexpectedFiles.length === 0) { @@ -305,21 +310,21 @@ function diffChangedFiles(changedFiles) { } return [ - `PR changes files outside ${REGISTRY_JSON_PATH}; core review required for non-registry-only PRs: ${unexpectedFiles.join(', ')}`, + `PR changes files outside the extension registries; core review required for registry-only PRs: ${unexpectedFiles.join(', ')}`, ]; } /** - * Fetches and parses cli/azd/extensions/registry.json at a given ref. + * Fetches and parses an extension registry at a given ref. * - * @param {{ octokit: Octokit, owner: string, repo: string, ref: string }} args + * @param {{ octokit: Octokit, owner: string, repo: string, ref: string, registryPath?: string }} args * @returns {Promise} */ -async function getRegistryJson({ octokit, owner, repo, ref }) { +async function getRegistryJson({ octokit, owner, repo, ref, registryPath = DEFAULT_REGISTRY_JSON_PATH }) { const { data } = await octokit.rest.repos.getContent({ owner, repo, - path: REGISTRY_JSON_PATH, + path: registryPath, ref, mediaType: { format: 'raw', @@ -327,7 +332,7 @@ async function getRegistryJson({ octokit, owner, repo, ref }) { }); if (typeof data !== 'string') { - throw new Error(`Unable to load ${REGISTRY_JSON_PATH} from ${owner}/${repo}@${ref}`); + throw new Error(`Unable to load ${registryPath} from ${owner}/${repo}@${ref}`); } return JSON.parse(data); @@ -803,5 +808,3 @@ function assertHasPullRequest(context) { throw new Error('No pull_request found in event payload. Workflow targeting should only target pull requests.'); } } - - diff --git a/.github/scripts/test/ext-registry-check.test.js b/.github/scripts/test/ext-registry-check.test.js index c154a6d4ae5..5bdfe84d49d 100644 --- a/.github/scripts/test/ext-registry-check.test.js +++ b/.github/scripts/test/ext-registry-check.test.js @@ -546,6 +546,21 @@ describe('isAllowedRegistryJsonUpdate', () => { })); }); + it('loads and validates registry.dev.json when requested', async () => { + const base = registry([extension({ id: 'ext.one' })]); + const pr = registry([{ ...extension({ id: 'ext.one' }), namespace: 'other' }]); + const octokit = createRegistryOctokit({ base, pr }); + + await expect(isAllowedRegistryJsonUpdate({ + octokit, + context: createRegistryContext(), + registryPath: 'cli/azd/extensions/registry.dev.json', + })).resolves.toContainEqual(expect.stringContaining('changes metadata that requires core review')); + expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ + path: 'cli/azd/extensions/registry.dev.json', + })); + }); + it('requires review when an existing release changes capabilities', async () => { const base = registry([extension({ id: 'ext.one', versions: [version({ capabilities: ['custom-commands'] })] })]); const pr = registry([extension({ id: 'ext.one', versions: [version({ capabilities: ['lifecycle-events'] })] })]); @@ -616,6 +631,54 @@ describe('run', () => { })); }); + it('allows a simple registry.dev.json-only PR without core review', async () => { + const core = createNoopCore(); + const octokit = createRegistryOctokit({ + base: registry([extension({ versions: [version({ version: '1.0.0' })] })]), + pr: registry([extension({ versions: [version({ version: '1.0.0' }), version({ version: '1.1.0' })] })]), + files: [{ filename: 'cli/azd/extensions/registry.dev.json' }], + }); + + await run({ + github: octokit, + context: createRegistryContext(), + core, + coreTeam: new Set(['core-member']), + }); + + expect(core.setFailed).not.toHaveBeenCalled(); + expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ + path: 'cli/azd/extensions/registry.dev.json', + })); + }); + + it('allows simple updates to both registries without core review', async () => { + const core = createNoopCore(); + const octokit = createRegistryOctokit({ + base: registry([extension({ versions: [version({ version: '1.0.0' })] })]), + pr: registry([extension({ versions: [version({ version: '1.0.0' }), version({ version: '1.1.0' })] })]), + files: [ + { filename: 'cli/azd/extensions/registry.json' }, + { filename: 'cli/azd/extensions/registry.dev.json' }, + ], + }); + + await run({ + github: octokit, + context: createRegistryContext(), + core, + coreTeam: new Set(['core-member']), + }); + + expect(core.setFailed).not.toHaveBeenCalled(); + expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ + path: 'cli/azd/extensions/registry.json', + })); + expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ + path: 'cli/azd/extensions/registry.dev.json', + })); + }); + it('skips changed-file review when a registry maintainer authored the PR', async () => { const core = createNoopCore(); const octokit = createRegistryOctokit({ @@ -687,7 +750,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside cli/azd/extensions/registry.json')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -745,7 +808,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside cli/azd/extensions/registry.json')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -817,7 +880,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside cli/azd/extensions/registry.json')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('cli/azd/extensions/README.md')); }); }); diff --git a/.github/workflows/ext-registry-check.yml b/.github/workflows/ext-registry-check.yml index 6af3e253221..2036d699548 100644 --- a/.github/workflows/ext-registry-check.yml +++ b/.github/workflows/ext-registry-check.yml @@ -35,7 +35,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: - - name: Check if registry.json changed + - name: Check if an extension registry changed id: changed uses: actions/github-script@v9 with: @@ -45,18 +45,24 @@ jobs: repo: context.repo.repo, pull_number: context.payload.pull_request.number, }); - const changed = files.some((f) => f.filename === 'cli/azd/extensions/registry.json'); - core.info(`registry.json changed: ${changed}`); - core.setOutput('registry_json_changed', changed); + const registryPaths = new Set([ + 'cli/azd/extensions/registry.json', + 'cli/azd/extensions/registry.dev.json', + ]); + const changed = files.some( + (f) => registryPaths.has(f.filename) || registryPaths.has(f.previous_filename), + ); + core.info(`extension registry changed: ${changed}`); + core.setOutput('registry_changed', changed); - name: Checkout - if: steps.changed.outputs.registry_json_changed == 'true' + if: steps.changed.outputs.registry_changed == 'true' uses: actions/checkout@v6 with: ref: ${{ github.event.pull_request.base.sha }} - name: Check extension registry update - if: steps.changed.outputs.registry_json_changed == 'true' + if: steps.changed.outputs.registry_changed == 'true' uses: actions/github-script@v9 with: script: | From f7becfca6b211667774e6c3fb7efca33cd55fc24 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Tue, 21 Jul 2026 20:56:01 +0000 Subject: [PATCH 2/3] Require explicit extension registry parameters Remove implicit registry path and base ref defaults, and include the supported registry paths in validation errors. --- .github/scripts/src/ext-registry-check.js | 14 +++++------ .../scripts/test/ext-registry-check.test.js | 25 ++++++++++++++----- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/.github/scripts/src/ext-registry-check.js b/.github/scripts/src/ext-registry-check.js index 80a5c58782f..86a8ac7b5fa 100644 --- a/.github/scripts/src/ext-registry-check.js +++ b/.github/scripts/src/ext-registry-check.js @@ -20,7 +20,6 @@ const REGISTRY_JSON_PATHS = new Set([ 'cli/azd/extensions/registry.json', 'cli/azd/extensions/registry.dev.json', ]); -const DEFAULT_REGISTRY_JSON_PATH = 'cli/azd/extensions/registry.json'; // We only allow URLs that point to our GitHub releases page. const ALLOWED_ARTIFACT_URL_ORIGIN = 'https://github.com'; @@ -244,14 +243,14 @@ function isCreatedByCoreTeam({ context, core, coreTeam }) { /** * Checks whether the registry update is simple enough to proceed without core-team review. * - * @param {{ octokit: Octokit, context: Context, registryBaseRef?: string, registryPath?: string }} args + * @param {{ octokit: Octokit, context: Context, registryPath: string, registryBaseRef: string }} args * @returns {Promise} the reasons core review is needed; empty means the change is approved */ async function isAllowedRegistryJsonUpdate({ octokit, context, - registryBaseRef = 'main', - registryPath = DEFAULT_REGISTRY_JSON_PATH, + registryPath, + registryBaseRef, }) { assertHasPullRequest(context); const pr = context.payload.pull_request; @@ -310,17 +309,18 @@ function diffChangedFiles(changedFiles) { } return [ - `PR changes files outside the extension registries; core review required for registry-only PRs: ${unexpectedFiles.join(', ')}`, + `PR changes files outside ${[...REGISTRY_JSON_PATHS].join(', ')}; ` + + `core review required for registry-only PRs: ${unexpectedFiles.join(', ')}`, ]; } /** * Fetches and parses an extension registry at a given ref. * - * @param {{ octokit: Octokit, owner: string, repo: string, ref: string, registryPath?: string }} args + * @param {{ octokit: Octokit, owner: string, repo: string, ref: string, registryPath: string }} args * @returns {Promise} */ -async function getRegistryJson({ octokit, owner, repo, ref, registryPath = DEFAULT_REGISTRY_JSON_PATH }) { +async function getRegistryJson({ octokit, owner, repo, ref, registryPath }) { const { data } = await octokit.rest.repos.getContent({ owner, repo, diff --git a/.github/scripts/test/ext-registry-check.test.js b/.github/scripts/test/ext-registry-check.test.js index 5bdfe84d49d..e45c2b85291 100644 --- a/.github/scripts/test/ext-registry-check.test.js +++ b/.github/scripts/test/ext-registry-check.test.js @@ -37,6 +37,10 @@ const { isAllowedRegistryJsonUpdate, } = run.forTests; +const PROD_REGISTRY_PATH = 'cli/azd/extensions/registry.json'; +const DEV_REGISTRY_PATH = 'cli/azd/extensions/registry.dev.json'; +const REGISTRY_PATH_LIST = `${PROD_REGISTRY_PATH}, ${DEV_REGISTRY_PATH}`; + /** * @param {object} [opts] * @param {string[]} [opts.capabilities] @@ -515,7 +519,12 @@ describe('isAllowedRegistryJsonUpdate', () => { const octokit = createRegistryOctokit({ base, pr }); const context = createRegistryContext(); - await expect(isAllowedRegistryJsonUpdate({ octokit, context })).resolves.toContainEqual(expect.stringContaining('changes metadata that requires core review')); + await expect(isAllowedRegistryJsonUpdate({ + octokit, + context, + registryPath: PROD_REGISTRY_PATH, + registryBaseRef: 'main', + })).resolves.toContainEqual(expect.stringContaining('changes metadata that requires core review')); expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ owner: 'Azure', repo: 'azure-dev', @@ -537,6 +546,7 @@ describe('isAllowedRegistryJsonUpdate', () => { await expect(isAllowedRegistryJsonUpdate({ octokit, context, + registryPath: PROD_REGISTRY_PATH, registryBaseRef: 'base-before-pr', })).resolves.toContainEqual(expect.stringContaining('changes metadata that requires core review')); expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ @@ -554,7 +564,8 @@ describe('isAllowedRegistryJsonUpdate', () => { await expect(isAllowedRegistryJsonUpdate({ octokit, context: createRegistryContext(), - registryPath: 'cli/azd/extensions/registry.dev.json', + registryPath: DEV_REGISTRY_PATH, + registryBaseRef: 'main', })).resolves.toContainEqual(expect.stringContaining('changes metadata that requires core review')); expect(octokit.rest.repos.getContent).toHaveBeenCalledWith(expect.objectContaining({ path: 'cli/azd/extensions/registry.dev.json', @@ -569,6 +580,8 @@ describe('isAllowedRegistryJsonUpdate', () => { const reasons = await isAllowedRegistryJsonUpdate({ octokit, context: createRegistryContext(), + registryPath: PROD_REGISTRY_PATH, + registryBaseRef: 'main', }); expect(reasons).toContainEqual(expect.stringContaining('changes capabilities')); @@ -750,7 +763,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -808,7 +821,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -862,7 +875,7 @@ describe('run', () => { })); }); - it('requires review when the PR changes files outside registry.json', async () => { + it('requires review when the PR changes files outside the extension registries', async () => { const core = createNoopCore(); const octokit = createRegistryOctokit({ base: registry([extension()]), @@ -880,7 +893,7 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('files outside the extension registries')); + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('cli/azd/extensions/README.md')); }); }); From 40743ce5f5d2ea32f4aec8d51d822cb32e4f8c62 Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Tue, 21 Jul 2026 21:18:17 +0000 Subject: [PATCH 3/3] Clarify blocked-file review messages and test registry isolation --- .github/scripts/src/ext-registry-check.js | 37 +++++-- .../scripts/test/ext-registry-check.test.js | 101 ++++++++++++++++-- 2 files changed, 121 insertions(+), 17 deletions(-) diff --git a/.github/scripts/src/ext-registry-check.js b/.github/scripts/src/ext-registry-check.js index 86a8ac7b5fa..adbcfbdc9ed 100644 --- a/.github/scripts/src/ext-registry-check.js +++ b/.github/scripts/src/ext-registry-check.js @@ -296,22 +296,41 @@ async function getChangedFiles({ octokit, context }) { } /** + * Flags file-level changes that disqualify a PR from the registry-only fast path. + * Only in-place edits to the known registry files may skip core review; touching any + * other file, or renaming a registry file, requires a core reviewer. + * * @param {PullRequestFile[]} changedFiles - * @returns {string[]} + * @returns {string[]} the reasons core review is needed; empty means every change is registry-only */ function diffChangedFiles(changedFiles) { - const unexpectedFiles = changedFiles - .filter((file) => !REGISTRY_JSON_PATHS.has(file.filename) || file.previous_filename != null) + const registryPaths = [...REGISTRY_JSON_PATHS].join(', '); + + const nonRegistryFiles = changedFiles + .filter((file) => !REGISTRY_JSON_PATHS.has(file.filename)) .map((file) => file.filename); - if (unexpectedFiles.length === 0) { - return []; + const renamedRegistryFiles = changedFiles + .filter((file) => REGISTRY_JSON_PATHS.has(file.filename) && file.previous_filename != null) + .map((file) => `${file.previous_filename} -> ${file.filename}`); + + const reasons = []; + + if (nonRegistryFiles.length > 0) { + reasons.push( + `PR changes files outside the extension registries (${registryPaths}), ` + + `which requires core review: ${nonRegistryFiles.join(', ')}`, + ); } - return [ - `PR changes files outside ${[...REGISTRY_JSON_PATHS].join(', ')}; ` + - `core review required for registry-only PRs: ${unexpectedFiles.join(', ')}`, - ]; + if (renamedRegistryFiles.length > 0) { + reasons.push( + `PR renames extension registry files, which requires core review: ` + + `${renamedRegistryFiles.join(', ')}`, + ); + } + + return reasons; } /** diff --git a/.github/scripts/test/ext-registry-check.test.js b/.github/scripts/test/ext-registry-check.test.js index e45c2b85291..5f1a4f10052 100644 --- a/.github/scripts/test/ext-registry-check.test.js +++ b/.github/scripts/test/ext-registry-check.test.js @@ -763,7 +763,8 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining(`files outside the extension registries (${REGISTRY_PATH_LIST})`)); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -821,7 +822,8 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining(`files outside the extension registries (${REGISTRY_PATH_LIST})`)); expect(octokit.paginate).toHaveBeenCalledWith(octokit.rest.pulls.listFiles, expect.objectContaining({ pull_number: 1, })); @@ -893,9 +895,71 @@ describe('run', () => { coreTeam: new Set(['core-member']), }); - expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`files outside ${REGISTRY_PATH_LIST}`)); + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining(`files outside the extension registries (${REGISTRY_PATH_LIST})`)); expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('cli/azd/extensions/README.md')); }); + + it('requires review when a registry file is renamed', async () => { + const core = createNoopCore(); + const octokit = createRegistryOctokit({ + base: registry([extension()]), + pr: registry([extension()]), + files: [ + { filename: PROD_REGISTRY_PATH, previous_filename: 'cli/azd/extensions/registry.old.json' }, + ], + }); + + await run({ + github: octokit, + context: createRegistryContext(), + core, + coreTeam: new Set(['core-member']), + }); + + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining('renames extension registry files')); + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining(`cli/azd/extensions/registry.old.json -> ${PROD_REGISTRY_PATH}`)); + }); + + it('evaluates each registry independently and names only the one that requires review', async () => { + const core = createNoopCore(); + const octokit = createRegistryOctokit({ + registries: { + // Prod registry: a simple, policy-valid new release. + [PROD_REGISTRY_PATH]: { + base: registry([extension({ versions: [version({ version: '1.0.0' })] })]), + pr: registry([extension({ versions: [version({ version: '1.0.0' }), version({ version: '1.1.0' })] })]), + }, + // Dev registry: a new release that changes capabilities, which requires core review. + [DEV_REGISTRY_PATH]: { + base: registry([extension({ versions: [version({ version: '1.0.0', capabilities: ['custom-commands'] })] })]), + pr: registry([ + extension({ + versions: [ + version({ version: '1.0.0', capabilities: ['custom-commands'] }), + version({ version: '1.1.0', capabilities: ['custom-commands', 'lifecycle-events'] }), + ], + }), + ]), + }, + }, + files: [ + { filename: PROD_REGISTRY_PATH }, + { filename: DEV_REGISTRY_PATH }, + ], + }); + + await run({ + github: octokit, + context: createRegistryContext(), + core, + coreTeam: new Set(['core-member']), + }); + + expect(core.setFailed).toHaveBeenCalledWith(expect.stringContaining(`${DEV_REGISTRY_PATH}: `)); + expect(core.setFailed).not.toHaveBeenCalledWith(expect.stringContaining(`${PROD_REGISTRY_PATH}: `)); + }); }); describe('getCoreReviewers', () => { @@ -912,10 +976,24 @@ describe('getCoreReviewers', () => { }); /** - * @param {{ base: RegistryJson, pr: RegistryJson, files?: { filename: string, previous_filename?: string }[], reviews?: { user: { login: string }, state: string, commit_id: string }[] }} args + * @param {{ + * base?: RegistryJson, + * pr?: RegistryJson, + * registries?: Object, + * files?: { filename: string, previous_filename?: string }[], + * reviews?: { user: { login: string }, state: string, commit_id: string }[], + * }} args * @returns {Octokit} */ -function createRegistryOctokit({ base, pr, files = [{ filename: 'cli/azd/extensions/registry.json' }], reviews = [] }) { +function createRegistryOctokit({ base, pr, registries, files = [{ filename: PROD_REGISTRY_PATH }], reviews = [] }) { + // Per-path fixtures let a test drive each registry independently. When they're not + // supplied, every registry path shares the same base/pr content. + /** @type {Record} */ + const registriesByPath = registries ?? { + [PROD_REGISTRY_PATH]: { base, pr }, + [DEV_REGISTRY_PATH]: { base, pr }, + }; + const octokit = { rest: { pulls: { @@ -923,9 +1001,16 @@ function createRegistryOctokit({ base, pr, files = [{ filename: 'cli/azd/extensi listFiles: vi.fn(), }, repos: { - getContent: vi.fn(({ ref }) => Promise.resolve({ - data: JSON.stringify(ref === 'abc123' ? pr : base), - })), + getContent: vi.fn(({ path, ref }) => { + const fixture = registriesByPath[path]; + if (fixture == null) { + throw new Error(`No registry fixture configured for ${path}`); + } + + return Promise.resolve({ + data: JSON.stringify(ref === 'abc123' ? fixture.pr : fixture.base), + }); + }), }, }, paginate: vi.fn((endpoint) => {