From 9c189a7894e62cd469cd9b817765186aed6f9b3e Mon Sep 17 00:00:00 2001 From: Senthanal Sirpi Manohar Date: Tue, 18 Aug 2026 21:48:32 +0200 Subject: [PATCH] fix(artifacts): resolve the snapshot existence check against JFrog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR snapshots are versioned `0.0.0--`, which is constant for the lifetime of a PR, so every push after the first republishes an identical version. NxProject.deleteArtifact compensates by unpublishing the version before publishing it again, but its guard never let that happen. packageExists ran `npm show --json` with no cwd and no --registry. From the repository root that resolves against the public registry, where the private scope does not exist, so the lookup 404'd, the guard read the 404 as "version absent", the unpublish was skipped, and npm publish then PUT a version that was already on JFrog — which Artifactory rejects with 403 Forbidden. The check now pins --registry to the JFrog URL the publish and unpublish already target, and runs in the dist directory whose .npmrc carries the credentials for it. The dist bootstrap in deleteArtifact moved above the check so the ONLY_DELETE_ARTIFACTS path is authenticated too. A second defect kept this invisible: the catch reported every failure as "not found", so an auth error or a registry outage produced the same silent skip and the same 403 one step later. Only a 404 now counts as absent; anything else fails the job with the underlying cause. Observed on cplace-dataciders-portfolio-management-fe PR #46, where the snapshot job succeeded once on 2026-08-06 and then failed identically on every subsequent push. Co-Authored-By: Claude Opus 5 --- tools/scripts/artifacts/nx-project.test.ts | 103 +++++++++++++++++++++ tools/scripts/artifacts/nx-project.ts | 83 +++++++++++++---- 2 files changed, 168 insertions(+), 18 deletions(-) diff --git a/tools/scripts/artifacts/nx-project.test.ts b/tools/scripts/artifacts/nx-project.test.ts index 4bedb5a2..183279e9 100644 --- a/tools/scripts/artifacts/nx-project.test.ts +++ b/tools/scripts/artifacts/nx-project.test.ts @@ -1,9 +1,13 @@ import { expect } from '@jest/globals'; +import { execSync } from 'child_process'; import * as fs from 'fs'; import { NxProject, NxProjectKind } from './nx-project'; import { globResult, packageJsonLib1 } from './test-data'; import { Utils } from './utils'; import { sep } from 'node:path'; +import { Version } from './version'; + +jest.mock('child_process'); afterEach(() => { jest.resetAllMocks(); @@ -113,3 +117,102 @@ test('regular app is always publishable regardless of public_api.ts', async () = expect(nxProject.isPublishable).toBe(true); }); + +describe('NxProject.deleteArtifact', () => { + const JFROG_REGISTRY = 'https://cplace.jfrog.io/artifactory/cplace-npm-local'; + const version = new Version('0.0.0', '-my-branch-46'); + + function anApp(): NxProject { + return new NxProject( + 'my-app', + NxProjectKind.Application, + undefined, + version, + '@cplace-next' + ); + } + + function npmShowFails(output: string) { + (execSync as jest.Mock).mockImplementationOnce(() => { + throw Object.assign(new Error('Command failed'), { + status: 1, + stderr: Buffer.from(output), + }); + }); + } + + beforeEach(() => { + jest.spyOn(fs, 'existsSync').mockReturnValue(true); + }); + + test('looks the version up in the JFrog registry, not the public one', async () => { + (execSync as jest.Mock).mockReturnValue( + Buffer.from( + JSON.stringify({ + name: '@cplace-next/my-app', + versions: ['0.0.0-my-branch-46'], + }) + ) + ); + + await anApp().deleteArtifact(version); + + const [command, options] = (execSync as jest.Mock).mock.calls[0]; + expect(command).toBe( + `npm show @cplace-next/my-app --json --registry=${JFROG_REGISTRY}` + ); + expect(options.cwd).toContain('dist/apps/my-app'.replace(/\//g, sep)); + }); + + test('unpublishes the version when the registry has it', async () => { + (execSync as jest.Mock).mockReturnValue( + Buffer.from( + JSON.stringify({ + name: '@cplace-next/my-app', + versions: ['0.0.0-my-branch-46'], + }) + ) + ); + + await anApp().deleteArtifact(version); + + expect((execSync as jest.Mock).mock.calls[1][0]).toBe( + 'npm unpublish @cplace-next/my-app@0.0.0-my-branch-46 --force' + ); + }); + + test('skips the deletion when the package is unknown to the registry', async () => { + npmShowFails('npm error code E404\nnpm error 404 Not Found - GET ...'); + + await anApp().deleteArtifact(version); + + expect((execSync as jest.Mock).mock.calls).toHaveLength(1); + }); + + test('skips the deletion when the registry knows other versions only', async () => { + (execSync as jest.Mock).mockReturnValue( + Buffer.from( + JSON.stringify({ + name: '@cplace-next/my-app', + versions: ['0.0.0-my-branch-45'], + }) + ) + ); + + await anApp().deleteArtifact(version); + + expect((execSync as jest.Mock).mock.calls).toHaveLength(1); + }); + + test('fails loudly instead of skipping when the lookup itself broke', async () => { + const exit = jest + .spyOn(process, 'exit') + .mockImplementation((() => undefined) as never); + npmShowFails('npm error code E500\nnpm error 500 Internal Server Error'); + + await anApp().deleteArtifact(version); + + expect((execSync as jest.Mock).mock.calls).toHaveLength(1); + expect(exit).toHaveBeenCalledWith(1); + }); +}); diff --git a/tools/scripts/artifacts/nx-project.ts b/tools/scripts/artifacts/nx-project.ts index 5b8a2031..5e861dd2 100644 --- a/tools/scripts/artifacts/nx-project.ts +++ b/tools/scripts/artifacts/nx-project.ts @@ -171,38 +171,70 @@ export class NxProject { } } - private packageExists(pkg: string, version: string) { + /** + * `npm show` has to be pinned to the JFrog registry explicitly. Run without + * `--registry` it resolves against whatever the current working directory's + * `.npmrc` configures, and from the repository root that is the public + * registry, where the private scope does not exist. The resulting 404 is + * indistinguishable from a genuinely absent version, so the deletion is + * skipped and the subsequent publish collides with the version that is + * actually there. + */ + private packageExists(pkg: string, version: string): boolean { + const registry = getJfrogUrl(); + const pathToProjectInDist = this.getPathToProjectInDist(); try { - const scopeSearchResult = execSync(`npm show ${pkg} --json`).toString(); + const scopeSearchResult = execSync( + `npm show ${pkg} --json --registry=${registry}`, + { + // The dist .npmrc carries the credentials for the registry above. + ...(fs.existsSync(pathToProjectInDist) + ? { cwd: pathToProjectInDist } + : {}), + stdio: ['ignore', 'pipe', 'pipe'], + } + ).toString(); console.log(`Search result from registry: ${scopeSearchResult}`); const npmPackage = JSON.parse(scopeSearchResult); console.log(`Package found in registry: ${npmPackage.name}`); console.log(`Package versions in registry: ${npmPackage.versions}`); return npmPackage.versions.includes(version); - } catch (e) { - console.log(`Package ${pkg} not found in registry.`); - return false; + } catch (error: any) { + if (NxProject.isMissingFromRegistry(error)) { + console.log(`Package ${pkg} not found in registry ${registry}.`); + return false; + } + throw new Error( + `Could not determine whether ${pkg}@${version} exists in ${registry}. ` + + `Refusing to skip the deletion, because publishing over an existing ` + + `version fails with a 403. Cause: ${ + error?.stderr?.toString() || error?.message || error + }` + ); } } + /** + * Only a 404 means the package or version is absent. Every other failure + * (auth, DNS, a registry outage) leaves the question unanswered and must not + * be reported as "does not exist". + */ + private static isMissingFromRegistry(error: any): boolean { + const output = [ + error?.stderr?.toString(), + error?.stdout?.toString(), + error?.message, + ] + .filter(Boolean) + .join('\n'); + return output.includes('E404') || output.includes('404 Not Found'); + } + public async deleteArtifact( version: Version, jfrogCredentials: JfrogCredentials = null ) { - console.log('Checking if package exists in registry'); const scopedPackage = `${this.scope}/${this.name}`; - if (!this.packageExists(scopedPackage, version.toString())) { - console.log( - `Package ${scopedPackage}@${version.toString()} does not exist in the registry. Skipping deletion.` - ); - return; - } - console.log( - `Package ${scopedPackage}@${version.toString()} exists in registry` - ); - console.log( - `About to delete artifact from Jfrog: ${this.name}@${version.toString()}` - ); try { const pathToProjectInDist = this.getPathToProjectInDist(); if (!fs.existsSync(pathToProjectInDist) && jfrogCredentials) { @@ -221,6 +253,21 @@ export class NxProject { ); console.log(`Generated package.json: ${this.getPrettyPackageJson()}`); } + // Has to run after the dist .npmrc exists, so the lookup is authenticated + // against JFrog rather than falling through to an anonymous 404. + console.log('Checking if package exists in registry'); + if (!this.packageExists(scopedPackage, version.toString())) { + console.log( + `Package ${scopedPackage}@${version.toString()} does not exist in the registry. Skipping deletion.` + ); + return; + } + console.log( + `Package ${scopedPackage}@${version.toString()} exists in registry` + ); + console.log( + `About to delete artifact from Jfrog: ${this.name}@${version.toString()}` + ); console.log( execSync( `npm unpublish ${this.scope}/${