Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 15, 2025

Problem

The summarize-checks workflow occasionally created duplicate "Next Steps to Merge" comments when multiple instances ran simultaneously. This happened because:

  1. Two workflow instances started at nearly the same time
  2. Both checked for existing comments and found none
  3. Both created new comments, resulting in confusing duplicates

Example of duplicate comments

Solution

Implemented a post-creation cleanup mechanism in the commentOrUpdate function that:

// After creating a new comment, check for race condition
const afterComments = getExistingComments(prNumber);
if (afterComments.containsMultiple(NSTM)) {
  deleteCommentWeAdded(newComment.id);
  updateOldestExistingComment();
}

Key Features

  • Deterministic resolution: Always keeps the oldest comment (lowest ID) to ensure both racing instances make identical decisions
  • Safe cleanup: Deletes the newly created duplicate and updates the pre-existing comment
  • Minimal overhead: Only activates when race condition is actually detected
  • Backward compatible: No changes to normal single-instance operation

Changes Made

  • Modified .github/workflows/src/comment.js to add race condition detection and cleanup
  • Added findAllMatchingComments() helper function to identify and sort duplicate comments
  • Fixed parseExistingComments() to return first match instead of last when multiple exist
  • Added comprehensive test suite covering all race condition scenarios

Test Coverage

Added 12 test cases including:

  • Normal comment creation and update flows
  • Race condition with 2 duplicate comments
  • Race condition with 3+ duplicate comments
  • Edge cases (empty arrays, null values, etc.)

This ensures users will never see duplicate "Next Steps to Merge" comments, even when multiple workflow instances run simultaneously.

Fixes #36683.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Add race condition detection and cleanup in commentOrUpdate function
- Keep oldest comment when duplicates are detected
- Delete newer duplicate comments automatically
- Add comprehensive test coverage for race condition scenarios
- Ensure deterministic behavior when multiple instances run simultaneously

Co-authored-by: mikeharder <[email protected]>
Copilot AI changed the title [WIP] [summarize-checks] Multiple NSTM comments race condition Fix race condition causing duplicate "Next Steps to Merge" comments Aug 15, 2025
Copilot AI requested a review from mikeharder August 15, 2025 18:11
Copy link
Member

@mikeharder mikeharder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot: Fix failing workflow "GitHub Actions - Test". Do not ask for another review, until all workflows named "GitHub Actions - Test" are passing.

@mikeharder
Copy link
Member

@copilot: Fix failing workflow "GitHub Actions - Test". Do not ask for another review, until all workflows named "GitHub Actions - Test" are passing.

Copy link
Contributor Author

Copilot AI commented Aug 15, 2025

@copilot: Fix failing workflow "GitHub Actions - Test". Do not ask for another review, until all workflows named "GitHub Actions - Test" are passing.

Fixed the failing "GitHub Actions - Test" workflow. The issue was TypeScript compilation errors due to missing type annotations in the findAllMatchingComments function. Also applied code formatting fixes. All tests are now passing. See commit 84ad30c.

@copilot: Fix failing workflow "GitHub Actions - Test". Do not ask for another review, until all workflows named "GitHub Actions - Test" are passing.

Fixed the failing "GitHub Actions - Test" workflow. The TypeScript compilation errors and formatting issues have been resolved. All tests, linting, and formatting checks are now passing. See commit 84ad30c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[summarize-checks] Multiple NSTM comments race condition

2 participants