diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index aa1675c957..14bf16837a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -889,7 +889,7 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" runtime-package: - name: Build and verify runtime release + name: Build SDK packages needs: [validate-dispatch, runtime-plan, runtime-acquire, runtime-test] if: | always() && @@ -966,7 +966,7 @@ jobs: retention-days: 30 runtime-publish-internal: - name: Publish runtime release internally + name: Publish SDK internally if: | always() && !cancelled() && @@ -1025,27 +1025,9 @@ jobs: run: | node nodejs/scripts/npm-release.js publish-manifest \ dist/release-manifest.json dist "${{ inputs.dist-tag }}" "$FEED_URL" azure - - name: Clean install and package version check - env: - SDK_VERSION: ${{ needs.runtime-plan.outputs.sdk_version }} - run: | - VERIFY_ROOT="$RUNNER_TEMP/sdk-${{ inputs.dist-tag }}-verification" - mkdir -p "$VERIFY_ROOT" - cd "$VERIFY_ROOT" - npm init -y >/dev/null - printf '%s\n' "@github:registry=${FEED_URL}" >> "$HOME/.npmrc" - npm install --ignore-scripts "@github/copilot-sdk@${SDK_VERSION}" - node -e ' - const expected = process.argv[1]; - const umbrella = require("./node_modules/@github/copilot-sdk/package.json"); - const platform = require("./node_modules/@github/copilot-sdk-linux-x64/package.json"); - if (umbrella.version !== expected || platform.version !== expected) { - throw new Error(`Installed SDK package version mismatch: ${umbrella.version}/${platform.version}, expected ${expected}`); - } - ' "$SDK_VERSION" runtime-publish-public: - name: Publish runtime release publicly + name: Publish SDK publicly if: | always() && !cancelled() && diff --git a/nodejs/scripts/npm-release.js b/nodejs/scripts/npm-release.js index 97a1d1a8fa..379f280202 100644 --- a/nodejs/scripts/npm-release.js +++ b/nodejs/scripts/npm-release.js @@ -78,7 +78,8 @@ export async function publishTarball(tarball, tag, registry, mode, runner = runC } const output = `${result.stdout}\n${result.stderr}`; - if (PUBLIC_CONFLICT.test(output) || (mode === "azure" && AZURE_CONFLICT.test(output))) { + const conflict = mode === "public" ? PUBLIC_CONFLICT : AZURE_CONFLICT; + if (conflict.test(output)) { const subject = identity?.name && identity?.version ? `${identity.name}@${identity.version}` @@ -117,42 +118,9 @@ export async function publishManifest( return left.name.localeCompare(right.name); }); - const semver = await import("semver"); - for (const packed of packages) { - const taggedVersion = await getRegistryVersion(packed.name, tag, registry, runner); - if (taggedVersion !== undefined && semver.gt(taggedVersion, packed.version)) { - throw new Error( - `${packed.name}@${tag} already points to newer version ${taggedVersion}; refusing to rewind it to ${packed.version}.` - ); - } - } for (const packed of packages) { await publishTarball(packed.tarball, tag, registry, mode, runner, packed); } - for (const packed of packages) { - const taggedVersion = await getRegistryVersion(packed.name, tag, registry, runner); - if (taggedVersion === packed.version) { - continue; - } - if (mode === "public") { - throw new Error( - `${packed.name}@${tag} resolves to ${taggedVersion ?? "no version"}, expected ${packed.version}. Public trusted publishing cannot repair dist-tags.` - ); - } - if (taggedVersion !== undefined && semver.gt(taggedVersion, packed.version)) { - throw new Error( - `${packed.name}@${tag} advanced to newer version ${taggedVersion}; refusing to rewind it to ${packed.version}.` - ); - } - const result = await runner( - "npm", - ["dist-tag", "add", `${packed.name}@${packed.version}`, tag, "--registry", registry], - { stream: true } - ); - if (result.status !== 0) { - throw new Error(`Failed to set ${packed.name}@${packed.version} dist-tag ${tag}.`); - } - } } async function main() { diff --git a/nodejs/test/npm-release.test.ts b/nodejs/test/npm-release.test.ts index 84326fcf21..f3d26da5df 100644 --- a/nodejs/test/npm-release.test.ts +++ b/nodejs/test/npm-release.test.ts @@ -1,7 +1,8 @@ import { createHash } from "node:crypto"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; import { describe, expect, it, vi } from "vitest"; import { assertVersionAbsent, publishManifest, publishTarball } from "../scripts/npm-release.js"; @@ -87,6 +88,12 @@ describe("npm release publishing", () => { "npm error network timeout while parsing \"already contains file 'package.tgz' in package '@github/copilot-sdk/1.2.3'\"", "azure", ], + ["a public conflict from Azure", "npm error code EPUBLISHCONFLICT", "azure"], + [ + "an Azure conflict from public npm", + "npm error 403 https://pkgs.dev.azure.com/example - The feed 'copilot-canary' already contains file 'package.tgz' in package '@github/copilot-sdk'.", + "public", + ], ["an unrelated npm failure", "npm error E500", "public"], ])("rejects %s", async (_name, error, mode) => { const runner = vi.fn().mockResolvedValue(result(1, "", error)); @@ -95,7 +102,7 @@ describe("npm release publishing", () => { ).rejects.toThrow("npm publish failed"); }); - it("validates all packages, publishes platforms before the umbrella, and tags last", async () => { + it("validates locally and publishes nine exact tarballs sequentially with the umbrella last", async () => { const directory = mkdtempSync(join(tmpdir(), "copilot-sdk-npm-release-")); mkdirSync(directory, { recursive: true }); const packages = [ @@ -127,89 +134,64 @@ describe("npm release publishing", () => { JSON.stringify({ schemaVersion: 1, sdk: { version }, packages }) ); const calls: string[][] = []; + let activePublishes = 0; + let maxActivePublishes = 0; const runner = vi.fn(async (_command: string, args: string[]) => { calls.push(args); - if (args[0] === "view") { - return result(0, JSON.stringify(version)); - } + activePublishes++; + maxActivePublishes = Math.max(maxActivePublishes, activePublishes); + await Promise.resolve(); + activePublishes--; return result(0); }); try { await publishManifest(manifestPath, directory, "unstable", registry, "public", runner); - const publishCalls = calls.filter((args) => args[0] === "publish"); - expect(publishCalls).toHaveLength(9); - expect(publishCalls.at(-1)?.[1]).toContain("package-0.tgz"); - expect(calls.filter((args) => args[0] === "dist-tag")).toHaveLength(0); - expect(calls.some((args) => args.includes("dist.integrity"))).toBe(false); + expect(calls).toHaveLength(9); + expect(calls.every((args) => args[0] === "publish")).toBe(true); + expect(calls.map((args) => args[1])).toEqual([ + ...packages.slice(1).map(({ filename }) => resolve(directory, filename)), + resolve(directory, packages[0].filename), + ]); + expect(maxActivePublishes).toBe(1); - const staleTagRunner = vi.fn(async (_command: string, args: string[]) => { - const name = args[1].slice(0, args[1].lastIndexOf("@")); - const packed = packages.find((candidate) => candidate.name === name)!; - return result( - 0, - JSON.stringify(args[2] === "version" ? "9.0.0-unstable.1" : packed.integrity) - ); - }); - await expect( - publishManifest( - manifestPath, - directory, - "unstable", - registry, - "public", - staleTagRunner - ) - ).rejects.toThrow("refusing to rewind"); - await expect( - publishManifest( - manifestPath, - directory, - "unstable", - registry, - "azure", - staleTagRunner - ) - ).rejects.toThrow("refusing to rewind"); - const azureConflictRunner = vi.fn(async (_command: string, args: string[]) => - args[0] === "view" - ? result(0, JSON.stringify(version)) - : result( - 1, - "", - "npm error 403 https://pkgs.dev.azure.com/example - The feed 'copilot-canary' already contains file 'package.tgz' in package '@github/copilot-sdk'." - ) - ); - await expect( - publishManifest( - manifestPath, - directory, - "unstable", - registry, - "azure", - azureConflictRunner - ) - ).resolves.toBeUndefined(); - expect( - azureConflictRunner.mock.calls.some(([, args]) => args.includes("dist.integrity")) - ).toBe(false); - const missingTagRunner = vi.fn(async (_command: string, args: string[]) => { - return args[0] === "view" - ? result(1, JSON.stringify({ error: { code: "E404" } })) - : result(0); - }); + writeFileSync(join(directory, packages[1].filename), "tampered"); + runner.mockClear(); await expect( - publishManifest( - manifestPath, - directory, - "unstable", - registry, - "public", - missingTagRunner - ) - ).rejects.toThrow("Public trusted publishing cannot repair dist-tags"); + publishManifest(manifestPath, directory, "unstable", registry, "public", runner) + ).rejects.toThrow(/Size mismatch|Integrity mismatch/); + expect(runner).not.toHaveBeenCalled(); } finally { rmSync(directory, { recursive: true, force: true }); } }); }); + +const workflow = readFileSync( + resolve(dirname(fileURLToPath(import.meta.url)), "../../.github/workflows/publish.yml"), + "utf8" +).replaceAll("\r\n", "\n"); + +function workflowJob(jobId: string): string { + const marker = ` ${jobId}:\n`; + const start = workflow.indexOf(marker); + if (start < 0) throw new Error(`Workflow job not found: ${jobId}`); + const rest = workflow.slice(start + marker.length); + const nextJob = rest.search(/^ [a-z0-9-]+:\n/m); + return nextJob < 0 ? rest : rest.slice(0, nextJob); +} + +describe("runtime-backed npm publishing workflow", () => { + it.each(["runtime-publish-internal", "runtime-publish-public"])( + "%s validates retained packages before using the shared publisher", + (jobId) => { + const job = workflowJob(jobId); + const validation = job.indexOf("- name: Validate retained release"); + const publication = job.indexOf("npm-release.js publish-manifest"); + + expect(validation).toBeGreaterThanOrEqual(0); + expect(publication).toBeGreaterThan(validation); + expect(job.match(/npm-release\.js publish-manifest/g)).toHaveLength(1); + } + ); +});