fix: duplicate PR fetch and spurious success-comment update (#187)

* fix: deduplicate PR fetch (#170) and gate success-comment on actual validation (#171)

Agent-Logs-Url: https://github.com/joshjohanning/azdo_commit_message_validator/sessions/b5e0234d-7619-4ff4-8ff5-6c9ba670b3d5

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

* refactor: return consistent {body} from all non-error paths in addWorkItemsToPRBody

Agent-Logs-Url: https://github.com/joshjohanning/azdo_commit_message_validator/sessions/b5e0234d-7619-4ff4-8ff5-6c9ba670b3d5

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

* chore: bump version to 4.1.3 and run npm run all

Agent-Logs-Url: https://github.com/joshjohanning/azdo_commit_message_validator/sessions/6a897449-d5cd-4f9d-8e44-c1e301e15c5b

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

* refactor: only extract prTitle when checkPullRequest is enabled

Agent-Logs-Url: https://github.com/joshjohanning/azdo_commit_message_validator/sessions/ad33ea36-d735-4c7c-a78c-b914a2e6da95

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

Author: Copilot

__tests__/index.test.js

@@ -2502,7 +2502,7 @@ describe('Azure DevOps Commit Validator', () => {
 
       await run();
 
-      // Should NOT call pulls.get or pulls.update since no IDs found
+      // Should NOT call pulls.update since no IDs found in branch
       expect(mockOctokit.rest.pulls.update).not.toHaveBeenCalled();
     });
 
@@ -2862,5 +2862,53 @@ describe('Azure DevOps Commit Validator', () => {
       expect(mockSetFailed).toHaveBeenCalledWith(expect.stringContaining('Personal Access Token (PAT) may be expired'));
       expect(mockOctokit.rest.pulls.update).not.toHaveBeenCalled();
     });
+
+    it('should NOT update invalid work item comment to success when only add-work-item-from-branch is enabled', async () => {
+      // Regression test for issue #171: the success-comment path must be gated on
+      // checkCommits || checkPullRequest so that branch-only runs don't incorrectly
+      // flip an existing "invalid work items" comment to success.
+      mockContext.payload.pull_request = { number: 42, head: { ref: 'task/12345/fix' } };
+
+      mockGetInput.mockImplementation(name => {
+        const inputs = {
+          'check-commits': 'false',
+          'check-pull-request': 'false',
+          'fail-if-missing-workitem-commit-link': 'false',
+          'link-commits-to-pull-request': 'false',
+          'comment-on-failure': 'true',
+          'validate-work-item-exists': 'true',
+          'add-work-item-from-branch': 'true',
+          'branch-work-item-prefixes': 'task, bug, bugfix',
+          'branch-work-item-min-digits': '5',
+          'github-token': 'github-token',
+          'azure-devops-token': 'fake-token',
+          'azure-devops-organization': 'my-org'
+        };
+        return inputs[name] || '';
+      });
+
+      mockOctokit.rest.pulls.get.mockResolvedValue({
+        data: { title: 'My PR', body: 'Description' }
+      });
+
+      // Branch work item 12345 exists
+      mockValidateWorkItemExists.mockResolvedValueOnce({ exists: true });
+
+      // Simulate an existing invalid work item comment from a previous run
+      mockOctokit.rest.issues.listComments.mockResolvedValue({
+        data: [
+          {
+            id: 555,
+            body: `${COMMENT_MARKERS.INVALID_WORK_ITEMS}\n:x: There is 1 work item in pull request #42 that does not exist in Azure DevOps.`
+          }
+        ]
+      });
+
+      await run();
+
+      expect(mockSetFailed).not.toHaveBeenCalled();
+      // The invalid work item comment must NOT be updated to success — no commit/PR validation ran
+      expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled();
+    });
   });
 });

badges/coverage.svg

@@ -1,6 +1,6 @@
 {
   "name": "azure-devops-work-item-link-enforcer-and-linker",
-  "version": "4.1.2",
+  "version": "4.1.3",
   "private": true,
   "type": "module",
   "description": "GitHub Action to enforce that each commit in a pull request be linked to an Azure DevOps work item and automatically link the pull request to each work item ",

package-lock.json

@@ -138,12 +138,29 @@ export async function run() {
 
     const octokit = github.getOctokit(githubToken);
 
+    // Fetch pull request data once, shared between addWorkItemsToPRBody and checkPullRequestForWorkItems
+    let prBody = '';
+    let prTitle = '';
+    if (addWorkItemFromBranch || checkPullRequest) {
+      const { owner, repo } = context.repo;
+      const pullRequestData = await octokit.rest.pulls.get({
+        owner,
+        repo,
+        pull_number: pullNumber
+      });
+      prBody = pullRequestData.data.body || '';
+      if (checkPullRequest) {
+        prTitle = pullRequestData.data.title || '';
+      }
+    }
+
     // Automatically add AB# tags from branch name if enabled
     if (addWorkItemFromBranch) {
       const branchResult = await addWorkItemsToPRBody(
         octokit,
         context,
         pullNumber,
+        prBody,
         azureDevopsOrganization,
         azureDevopsToken,
         branchWorkItemPrefixes,
@@ -154,6 +171,11 @@ export async function run() {
       if (branchResult && branchResult.authError) {
         return;
       }
+
+      // Use updated body for subsequent checks (e.g. checkPullRequest sees the newly added AB# tags)
+      if (branchResult?.body !== undefined) {
+        prBody = branchResult.body;
+      }
     }
 
     // Store work item to commit mapping and validation results
@@ -190,6 +212,8 @@ export async function run() {
         octokit,
         context,
         pullNumber,
+        prBody,
+        prTitle,
         commentOnFailure,
         validateWorkItemExistsFlag,
         azureDevopsOrganization,
@@ -247,7 +271,7 @@ export async function run() {
       core.setFailed(
         `There ${allInvalidWorkItems.length === 1 ? 'is' : 'are'} ${allInvalidWorkItems.length} work item${allInvalidWorkItems.length === 1 ? '' : 's'} that ${allInvalidWorkItems.length === 1 ? 'does' : 'do'} not exist in Azure DevOps`
       );
-    } else if (commentOnFailure && validateWorkItemExistsFlag) {
+    } else if (commentOnFailure && validateWorkItemExistsFlag && (checkCommits || checkPullRequest)) {
       // All work items are valid - check if there's an existing invalid work item comment to update to success
       const { owner, repo } = context.repo;
       const comments = await octokit.paginate(octokit.rest.issues.listComments, {
@@ -512,6 +536,8 @@ async function checkCommitsForWorkItems(
  * @param {Object} octokit - GitHub API client
  * @param {Object} context - GitHub Actions context
  * @param {number} pullNumber - Pull request number
+ * @param {string} pullBody - Current PR body text (pre-fetched by caller)
+ * @param {string} pullTitle - Current PR title (pre-fetched by caller)
  * @param {boolean} commentOnFailure - Whether to comment on PR if validation fails
  * @param {boolean} validateWorkItemExistsFlag - Whether to validate work items exist in Azure DevOps
  * @param {string} azureDevopsOrganization - Azure DevOps organization name
@@ -525,6 +551,8 @@ async function checkPullRequestForWorkItems(
   octokit,
   context,
   pullNumber,
+  pullBody,
+  pullTitle,
   commentOnFailure,
   validateWorkItemExistsFlag,
   azureDevopsOrganization,
@@ -535,16 +563,6 @@ async function checkPullRequestForWorkItems(
 ) {
   const { owner, repo } = context.repo;
 
-  // Get pull request details
-  const pullRequest = await octokit.rest.pulls.get({
-    owner,
-    repo,
-    pull_number: pullNumber
-  });
-
-  const pullBody = pullRequest.data.body || '';
-  const pullTitle = pullRequest.data.title || '';
-
   // Determine which text to check based on pull-request-check-scope
   let textToCheck;
   let scopeDescription;
@@ -840,16 +858,18 @@ export function extractWorkItemIdsFromBranch(branchName, prefixes, minDigits = 1
  * @param {Object} octokit - GitHub API client
  * @param {Object} context - GitHub Actions context
  * @param {number} pullNumber - Pull request number
+ * @param {string} currentBody - Current PR body text (pre-fetched by caller)
  * @param {string} azureDevopsOrganization - Azure DevOps organization name
  * @param {string} azureDevopsToken - Azure DevOps PAT token
  * @param {string[]} branchPrefixes - Keyword prefixes for identifying work item IDs in branch names
  * @param {number} [minDigits=1] - Minimum number of digits for a work item ID
- * @returns {Promise<{authError: true}|undefined>} - Auth error status if auth failed, undefined otherwise
+ * @returns {Promise<{body: string}|{authError: true}>} - Final PR body (updated or unchanged), or auth error status
  */
 async function addWorkItemsToPRBody(
   octokit,
   context,
   pullNumber,
+  currentBody,
   azureDevopsOrganization,
   azureDevopsToken,
   branchPrefixes,
@@ -865,7 +885,7 @@ async function addWorkItemsToPRBody(
 
   if (workItemIds.length === 0) {
     core.info('No work item IDs found in branch name');
-    return;
+    return { body: currentBody };
   }
 
   // Cap the number of IDs to validate to avoid excessive API calls
@@ -879,15 +899,6 @@ async function addWorkItemsToPRBody(
 
   core.info(`Found work item ID(s) in branch: ${workItemIds.join(', ')}`);
 
-  // Get current PR body
-  const pullRequest = await octokit.rest.pulls.get({
-    owner,
-    repo,
-    pull_number: pullNumber
-  });
-
-  const currentBody = pullRequest.data.body || '';
-
   // Filter to only IDs not already in the PR body
   const missingIds = workItemIds.filter(id => {
     const pattern = new RegExp(`AB#${id}(?!\\d)`, 'i');
@@ -896,7 +907,7 @@ async function addWorkItemsToPRBody(
 
   if (missingIds.length === 0) {
     core.info('All work item IDs from branch are already in the PR body');
-    return;
+    return { body: currentBody };
   }
 
   // Validate IDs against Azure DevOps before adding
@@ -922,7 +933,7 @@ async function addWorkItemsToPRBody(
   idsToAdd = validatedIds;
   if (idsToAdd.length === 0) {
     core.info('No valid work item IDs from branch to add (all failed validation)');
-    return;
+    return { body: currentBody };
   }
 
   // Build the AB# tags to add
@@ -939,6 +950,7 @@ async function addWorkItemsToPRBody(
   core.info('PR body updated with work item tag(s) from branch name');
   const sanitizedBranchName = branchName.replace(/\\/g, '\\\\').replace(/`/g, '\\`');
   core.summary.addRaw(`- :link: **Added from branch:** ${abTags} extracted from branch \`${sanitizedBranchName}\`\n`);
+  return { body: updatedBody };
 }
 
 /**