From ee89a333db291cb93cff745b3f96b3de13bb016b Mon Sep 17 00:00:00 2001 From: Senthanal Sirpi Manohar Date: Thu, 17 Sep 2026 22:20:09 +0200 Subject: [PATCH 1/2] feat(artifacts): skip tagging and publishing for [branch-off] commits cf-devkit's branch-off publishes the release artifacts locally and no longer creates any tags. Instead it marks every commit it makes with the literal subject prefix [branch-off]. Without a matching guard, CI sees the pushed commit on the release branch, re-tags it and republishes the same artifacts a second time. Add Utils.isBranchOffCommit(), which reads the subject of the checked out tip commit and matches the marker byte-for-byte at the start of the subject. Guard the artifacts entry point with it, so that all five workflows routing through the artifacts composite action are covered by a single check. The subject is read with git log rather than from github.event.head_commit.message, because the tag job checks out inputs.GHA_REF, which may differ from the SHA in the event payload. The tip CI actually evaluates is the one the guard has to inspect. PFM-ISSUE-34805 Co-Authored-By: Claude Opus 5 (1M context) --- tools/scripts/artifacts/main.ts | 12 +++++++++- tools/scripts/artifacts/utils.test.ts | 33 +++++++++++++++++++++++++++ tools/scripts/artifacts/utils.ts | 17 ++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/tools/scripts/artifacts/main.ts b/tools/scripts/artifacts/main.ts index cbab08e8..c61434cd 100644 --- a/tools/scripts/artifacts/main.ts +++ b/tools/scripts/artifacts/main.ts @@ -1,3 +1,13 @@ import { ArtifactsHandler } from './artifacts-handler'; +import { Utils } from './utils'; -new ArtifactsHandler().handle(); +// cf-devkit's branch-off bumps versions and publishes the artifacts locally, and +// marks every commit it creates. Tagging and publishing such a commit from CI +// would re-release what branch-off already released. +if (Utils.isBranchOffCommit()) { + console.log( + `Tip commit is marked with ${Utils.BRANCH_OFF_MARKER}, skipping tagging and publishing` + ); +} else { + new ArtifactsHandler().handle(); +} diff --git a/tools/scripts/artifacts/utils.test.ts b/tools/scripts/artifacts/utils.test.ts index e9cace86..e628726f 100644 --- a/tools/scripts/artifacts/utils.test.ts +++ b/tools/scripts/artifacts/utils.test.ts @@ -350,3 +350,36 @@ test('getAffectedNxProjects excludes e2e apps without public_api.ts', async () = expect(apps[0].name).toBe(app1); expect(apps[1].name).toBe(app2); }); + +test('isBranchOffCommit detects the branch-off marker on the tip commit', () => { + jest + .spyOn(child_process, 'execSync') + .mockReturnValueOnce(Buffer.from(rootDir)) + .mockReturnValueOnce( + Buffer.from('[branch-off] Branch off: Release 26.4\n') + ); + + expect(Utils.isBranchOffCommit()).toBe(true); +}); + +test('isBranchOffCommit ignores the marker outside of the subject prefix', () => { + jest + .spyOn(child_process, 'execSync') + .mockReturnValueOnce(Buffer.from(rootDir)) + .mockReturnValueOnce( + Buffer.from('fix: match the [branch-off] marker byte-for-byte\n') + ); + + expect(Utils.isBranchOffCommit()).toBe(false); +}); + +test('isBranchOffCommit is false for a regular commit', () => { + jest + .spyOn(child_process, 'execSync') + .mockReturnValueOnce(Buffer.from(rootDir)) + .mockReturnValueOnce( + Buffer.from('PFM-TASK-8334 - Roll out Node 24.20.0\n') + ); + + expect(Utils.isBranchOffCommit()).toBe(false); +}); diff --git a/tools/scripts/artifacts/utils.ts b/tools/scripts/artifacts/utils.ts index 4febcc38..42c418f9 100644 --- a/tools/scripts/artifacts/utils.ts +++ b/tools/scripts/artifacts/utils.ts @@ -13,6 +13,8 @@ export class Utils { public static readonly GITHUB_COMMENTS_FILE = 'githubCommentsForPR.txt'; public static readonly EMPTY_GITHUB_COMMENTS = 'No snapshots of projects have been published (probably no project is affected)'; + // cf-devkit's branch-off marks every commit it creates with this literal prefix. + public static readonly BRANCH_OFF_MARKER = '[branch-off]'; public static globProjectJSON(): string[] { const projects = execSync('ls */**/project.json').toString().trim(); @@ -231,6 +233,21 @@ export class Utils { return execSync(`git rev-parse --show-toplevel`).toString().trim(); } + /** + * True if the checked out tip commit was created by cf-devkit's branch-off. + * Branch-off bumps versions and publishes the artifacts itself, so CI must not + * tag and publish the same commit a second time. + */ + public static isBranchOffCommit(): boolean { + const subject = execSync(`git log -1 --pretty=%s`, { + cwd: Utils.getRootDir(), + }) + .toString() + .trim(); + console.log(`The subject of the tip commit is '${subject}'`); + return subject.startsWith(Utils.BRANCH_OFF_MARKER); + } + public static getLibsDir() { return path.resolve(this.getRootDir(), 'libs'); } From 96350765ff11f4306c126303df31d0ed44399d40 Mon Sep 17 00:00:00 2001 From: Senthanal Sirpi Manohar Date: Mon, 21 Sep 2026 17:20:29 +0200 Subject: [PATCH 2/2] fix(artifacts): scope the branch-off guard to tagging and publishing The guard was placed at module level in main.ts, which is the single entry point for all five workflows that use the artifacts composite action. That suppressed the whole action rather than only tagging and publishing, taking two unrelated behaviours down with it: - Utils.initGithubActionsFile() is the first statement of handle(), so githubCommentsForPR.txt was never created. The "comment published Artifacts on PR" step of fe-pr-snapshot.yml reads that file unconditionally and would have failed with ENOENT on a [branch-off] tip commit. - Artifact deletion (fe-pr-close, ONLY_DELETE_ARTIFACTS=true) never tags and never publishes, but was skipped as well, leaving the PR snapshot in the registry indefinitely. The guard decision depends on the run mode, which is parsed in the ArtifactsHandler constructor, so it cannot be made mode-aware above that point. Move it into handle(), after initGithubActionsFile() and gated on !this.onlyDeleteArtifacts. Note that onlyDeleteArtifacts is evaluated per project inside buildAndPublishProjects() rather than as a branch of handle(), so relocating the guard alone would not have restored deletion; the explicit test is required. Also document that isBranchOffCommit() reads the subject of whatever commit the workflow checked out, and would silently never fire if a caller switched to the pull request merge ref. Tests: four new cases in artifacts-handler-branch-off.test.ts cover the suppressed path, the comments file still being created, deletion still running, and the unchanged regular path. The existing handler suites mock execSync with a positional chain, so they now stub Utils.isBranchOffCommit to keep the guard from consuming two entries of that chain. Co-Authored-By: Claude Opus 5 (1M context) --- .../artifacts-handler-branch-off.test.ts | 74 +++++++++++++++++++ .../artifacts/artifacts-handler-tag.test.ts | 1 + .../artifacts/artifacts-handler.test.ts | 1 + tools/scripts/artifacts/artifacts-handler.ts | 13 ++++ tools/scripts/artifacts/main.ts | 12 +-- tools/scripts/artifacts/utils.ts | 6 ++ 6 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 tools/scripts/artifacts/artifacts-handler-branch-off.test.ts diff --git a/tools/scripts/artifacts/artifacts-handler-branch-off.test.ts b/tools/scripts/artifacts/artifacts-handler-branch-off.test.ts new file mode 100644 index 00000000..67baa2e1 --- /dev/null +++ b/tools/scripts/artifacts/artifacts-handler-branch-off.test.ts @@ -0,0 +1,74 @@ +import { expect } from '@jest/globals'; +import * as child_process from 'child_process'; +import { ArtifactsHandler } from './artifacts-handler'; +import { Utils } from './utils'; + +process.env.JFROG_BASE64_TOKEN = 'jfrog_base64_token'; +process.env.JFROG_URL = 'jfrog_url'; +process.env.JFROG_USER = 'jfrog_user'; + +afterEach(() => { + jest.resetAllMocks(); + jest.clearAllMocks(); + process.env.ONLY_BUMP_VERSION = 'false'; + process.env.ONLY_DELETE_ARTIFACTS = 'false'; + process.env.TAG = ''; + process.env.BASE = ''; + process.env.PR_NUMBER = ''; +}); + +test('on a [branch-off] tip commit nothing is built, tagged or published', async () => { + mockHandlerDependencies(true); + const affectedProjectsMock = jest + .spyOn(Utils, 'getAffectedNxProjects') + .mockReturnValue([]); + + await new ArtifactsHandler().handle(); + + expect(affectedProjectsMock).not.toBeCalled(); +}); + +test('the comments file is still created on a [branch-off] tip commit', async () => { + const initFileMock = mockHandlerDependencies(true); + + await new ArtifactsHandler().handle(); + + // fe-pr-snapshot.yml reads githubCommentsForPR.txt unconditionally; without + // this the comment step fails with ENOENT. + expect(initFileMock).toBeCalledTimes(1); +}); + +test('artifact deletion still runs on a [branch-off] tip commit', async () => { + process.env.ONLY_DELETE_ARTIFACTS = 'true'; + mockHandlerDependencies(true); + const affectedProjectsMock = jest + .spyOn(Utils, 'getAffectedNxProjects') + .mockReturnValue([]); + + await new ArtifactsHandler().handle(); + + expect(affectedProjectsMock).toBeCalled(); +}); + +test('a regular tip commit is built and published as before', async () => { + mockHandlerDependencies(false); + const affectedProjectsMock = jest + .spyOn(Utils, 'getAffectedNxProjects') + .mockReturnValue([]); + + await new ArtifactsHandler().handle(); + + expect(affectedProjectsMock).toBeCalled(); +}); + +function mockHandlerDependencies(isBranchOff: boolean) { + jest.spyOn(child_process, 'execSync').mockReturnValue(Buffer.from('')); + jest + .spyOn(Utils, 'getCurrentBranchNameFromGithubEnv') + .mockReturnValue('feature/PFM-ISSUE-1234-some-work'); + jest + .spyOn(Utils, 'parseScopeFromPackageJson') + .mockReturnValue('@cplace-frontend-applications'); + jest.spyOn(Utils, 'isBranchOffCommit').mockReturnValue(isBranchOff); + return jest.spyOn(Utils, 'initGithubActionsFile').mockReturnValue(); +} diff --git a/tools/scripts/artifacts/artifacts-handler-tag.test.ts b/tools/scripts/artifacts/artifacts-handler-tag.test.ts index 103fb52a..8d0fc214 100644 --- a/tools/scripts/artifacts/artifacts-handler-tag.test.ts +++ b/tools/scripts/artifacts/artifacts-handler-tag.test.ts @@ -109,5 +109,6 @@ function mockProjects() { .spyOn(Utils, 'getAllNxProjects') .mockReturnValueOnce([new NxProject('lib a', NxProjectKind.Library)]); jest.spyOn(Utils, 'initGithubActionsFile').mockReturnValue(); + jest.spyOn(Utils, 'isBranchOffCommit').mockReturnValue(false); jest.spyOn(Utils, 'getRootDir').mockReturnValue(rootDir); } diff --git a/tools/scripts/artifacts/artifacts-handler.test.ts b/tools/scripts/artifacts/artifacts-handler.test.ts index 92633936..12474227 100644 --- a/tools/scripts/artifacts/artifacts-handler.test.ts +++ b/tools/scripts/artifacts/artifacts-handler.test.ts @@ -172,6 +172,7 @@ async function exec() { .spyOn(Utils, 'parseScopeFromPackageJson') .mockReturnValueOnce('@cplace-frontend-applications'); jest.spyOn(Utils, 'initGithubActionsFile').mockReturnValue(); + jest.spyOn(Utils, 'isBranchOffCommit').mockReturnValue(false); jest .spyOn(Utils, 'writePublishedProjectToGithubCommentsFile') .mockReturnValue(); diff --git a/tools/scripts/artifacts/artifacts-handler.ts b/tools/scripts/artifacts/artifacts-handler.ts index d9434577..cd886266 100644 --- a/tools/scripts/artifacts/artifacts-handler.ts +++ b/tools/scripts/artifacts/artifacts-handler.ts @@ -98,7 +98,20 @@ export class ArtifactsHandler { } async handle() { + // Must run before any early return: the "comment published Artifacts on PR" + // step of fe-pr-snapshot.yml reads the comments file unconditionally. Utils.initGithubActionsFile(); + // cf-devkit's branch-off bumps versions and publishes the artifacts locally, + // and marks every commit it creates. Tagging and publishing such a commit + // from CI would re-release what branch-off already released. Deletion + // (fe-pr-close) stays unguarded: it neither tags nor publishes, and skipping + // it would leak the PR snapshot into the registry forever. + if (!this.onlyDeleteArtifacts && Utils.isBranchOffCommit()) { + console.log( + `Tip commit is marked with ${Utils.BRANCH_OFF_MARKER}, skipping tagging and publishing` + ); + return Promise.resolve(); + } if ( this.onlyBumpVersion && this.currentBranch.startsWith(ArtifactsHandler.RELEASE_BRANCH_PREFIX) diff --git a/tools/scripts/artifacts/main.ts b/tools/scripts/artifacts/main.ts index c61434cd..cbab08e8 100644 --- a/tools/scripts/artifacts/main.ts +++ b/tools/scripts/artifacts/main.ts @@ -1,13 +1,3 @@ import { ArtifactsHandler } from './artifacts-handler'; -import { Utils } from './utils'; -// cf-devkit's branch-off bumps versions and publishes the artifacts locally, and -// marks every commit it creates. Tagging and publishing such a commit from CI -// would re-release what branch-off already released. -if (Utils.isBranchOffCommit()) { - console.log( - `Tip commit is marked with ${Utils.BRANCH_OFF_MARKER}, skipping tagging and publishing` - ); -} else { - new ArtifactsHandler().handle(); -} +new ArtifactsHandler().handle(); diff --git a/tools/scripts/artifacts/utils.ts b/tools/scripts/artifacts/utils.ts index 42c418f9..9498be94 100644 --- a/tools/scripts/artifacts/utils.ts +++ b/tools/scripts/artifacts/utils.ts @@ -237,6 +237,12 @@ export class Utils { * True if the checked out tip commit was created by cf-devkit's branch-off. * Branch-off bumps versions and publishes the artifacts itself, so CI must not * tag and publish the same commit a second time. + * + * Reads the subject of whatever commit the workflow checked out, so it depends + * on the checkout ref of the caller. Every current caller checks out a real + * branch head or SHA (head.ref, head.sha, inputs.GHA_REF). A caller switching + * to the pull request merge ref would see the subject `Merge x into y` and the + * guard would silently never fire. */ public static isBranchOffCommit(): boolean { const subject = execSync(`git log -1 --pretty=%s`, {