fix(worker/repository): only set artifacts status when branch was updated (#44597)

* fix(worker/repository): only set artifacts status when branch was updated

* test: fix test

Author: viceice

lib/workers/repository/update/branch/index.spec.ts

@@ -37,6 +37,7 @@ import * as _changelog from '../../changelog/index.ts';
 import * as _prAutomerge from '../pr/automerge.ts';
 import type { ResultWithPr } from '../pr/index.ts';
 import * as _prWorker from '../pr/index.ts';
+import * as artifacts from './artifacts.ts';
 import * as _automerge from './automerge.ts';
 import * as _checkExisting from './check-existing.ts';
 import * as _commit from './commit.ts';
@@ -949,7 +950,11 @@ describe('workers/repository/update/branch/index', () => {
     });
 
     it('returns if branch automerge is pending', async () => {
-      expect.assertions(1);
+      expect.assertions(2);
+      const setArtifactErrorStatus = vi.spyOn(
+        artifacts,
+        'setArtifactErrorStatus',
+      );
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
         partial<PackageFilesResult>({
           updatedPackageFiles: [partial<FileChange>()],
@@ -972,6 +977,7 @@ describe('workers/repository/update/branch/index', () => {
         result: 'done',
         commitSha: null,
       });
+      expect(setArtifactErrorStatus).not.toHaveBeenCalled();
     });
 
     it('returns if PR creation failed', async () => {
@@ -1027,6 +1033,10 @@ describe('workers/repository/update/branch/index', () => {
     });
 
     it('retries setting branch status checks after PR creation', async () => {
+      const setArtifactErrorStatus = vi.spyOn(
+        artifacts,
+        'setArtifactErrorStatus',
+      );
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
         partial<PackageFilesResult>({
           updatedPackageFiles: [partial<FileChange>()],
@@ -1056,6 +1066,7 @@ describe('workers/repository/update/branch/index', () => {
       });
       // Called twice: once before ensurePr, once after PR creation
       expect(platform.setBranchStatus).toHaveBeenCalledTimes(2);
+      expect(setArtifactErrorStatus).not.toHaveBeenCalled();
     });
 
     it('does not retry setting branch status checks when PR is not created', async () => {
@@ -1091,7 +1102,11 @@ describe('workers/repository/update/branch/index', () => {
     });
 
     it('returns if branch exists but updated', async () => {
-      expect.assertions(3);
+      expect.assertions(4);
+      const setArtifactErrorStatus = vi.spyOn(
+        artifacts,
+        'setArtifactErrorStatus',
+      );
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
         partial<PackageFilesResult>({
           updatedPackageFiles: [partial<FileChange>()],
@@ -1119,10 +1134,15 @@ describe('workers/repository/update/branch/index', () => {
 
       expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
+      expect(setArtifactErrorStatus).toHaveBeenCalledTimes(1);
     });
 
     it('updates branch when no fingerprint match', async () => {
-      expect.assertions(3);
+      expect.assertions(4);
+      const setArtifactErrorStatus = vi.spyOn(
+        artifacts,
+        'setArtifactErrorStatus',
+      );
       getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce(
         partial<PackageFilesResult>({
           updatedPackageFiles: [partial<FileChange>()],
@@ -1151,6 +1171,7 @@ describe('workers/repository/update/branch/index', () => {
 
       expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(0);
       expect(prWorker.ensurePr).toHaveBeenCalledTimes(0);
+      expect(setArtifactErrorStatus).toHaveBeenCalledTimes(1);
     });
 
     it('updates branch when forceRebase=true', async () => {

lib/workers/repository/update/branch/index.ts

@@ -60,7 +60,6 @@ import { isScheduledNow } from './schedule.ts';
 import { setConfidence, setStability } from './status-checks.ts';
 
 async function setBranchStatusChecks(config: BranchConfig): Promise<void> {
-  await setArtifactErrorStatus(config);
   await setStability(config);
   await setConfidence(config);
 }
@@ -147,6 +146,7 @@ export async function processBranch(
       config.branchPrefixOld!,
     );
     branchExists = await scm.branchExists(branchName);
+    // v8 ignore else -- TODO: add test #40625
     if (branchExists) {
       config.branchName = branchName;
       logger.debug('Found existing branch with branchPrefixOld');
@@ -706,6 +706,7 @@ export async function processBranch(
             topic: artifactErrorTopic,
           });
 
+          // v8 ignore else -- TODO: add test #40625
           if (!config.artifactNotices?.length) {
             await ensureCommentRemoval({
               type: 'by-topic',
@@ -742,6 +743,12 @@ export async function processBranch(
       // baseBranch is not checked out at the start of processBranch() due to pull/16246
       await scm.checkoutBranch(config.baseBranch);
       updatesVerified = true;
+
+      // only update artifact status if branch was updated
+      // also do before platform automerge reattempt
+      if (commitSha) {
+        await setArtifactErrorStatus(config);
+      }
     }
 
     if (branchPr) {
@@ -762,6 +769,7 @@ export async function processBranch(
           });
         }
       }
+      // v8 ignore else -- TODO: add test #40625
       if (platform.refreshPr) {
         await platform.refreshPr(branchPr.number);
       }
@@ -1001,6 +1009,7 @@ export async function processBranch(
         commitSha,
       };
     }
+    // v8 ignore else -- TODO: add test #40625
     if (ensurePrResult.type === 'with-pr') {
       const { pr } = ensurePrResult;
       branchPr = pr;
@@ -1009,6 +1018,11 @@ export async function processBranch(
       // associate the status with. The earlier call may have been
       // skipped if no pipeline existed yet.
       await setBranchStatusChecks(config);
+
+      // only update artifact status if branch was updated
+      if (commitSha) {
+        await setArtifactErrorStatus(config);
+      }
       if (config.artifactErrors?.length) {
         logger.warn(
           { artifactErrors: config.artifactErrors },
@@ -1039,6 +1053,7 @@ export async function processBranch(
           content += `\`\`\`\n${error.stderr!}\n\`\`\`\n\n`;
         });
         content = platform.massageMarkdown(content, config.rebaseLabel);
+        // v8 ignore else -- TODO: add test #40625
         if (
           !(
             config.suppressNotifications!.includes('artifactErrors') ||
@@ -1075,6 +1090,7 @@ export async function processBranch(
         if (config.automerge) {
           logger.debug('PR is configured for automerge');
           // skip automerge if there is a new commit since status checks aren't done yet
+          // v8 ignore else -- TODO: add test #40625
           if (config.ignoreTests === true || !commitSha) {
             logger.debug('checking auto-merge');
             const prAutomergeResult = await checkAutoMerge(pr, config);