fix(ci): use temp lane names in ci pr to avoid race conditions#10183
Merged
Conversation
When multiple CI jobs run `bit ci pr` on the same branch concurrently, a race condition could cause one job to fail with 'lane exists with different hash'. This happened because the long snap operation occurred after lane creation, creating a large window for conflicts. The fix uses a temporary lane name with a random suffix during snap, then renames to the final name right before export. This minimizes the race window to just the quick delete-rename-export operations.
- Use crypto-based generateRandomStr instead of Math.random() for better collision resistance - Add throwOnError param to archiveLane for critical pre-export cleanup - Clean up temp lane on 'no changes detected' early return
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a race condition that occurs when multiple CI jobs run bit ci pr concurrently on the same branch. The original implementation would delete the existing lane, create a new one with the final name, and then perform a long snap operation (30+ minutes). If a second job started during this window, both would try to export to the same lane, causing a "lane exists with different hash" error.
Changes:
- Modified the
snapPrCommitmethod to use temporary lane names with random suffixes during the snap operation - Added cleanup logic for the temporary lane when no changes are detected
- Updated the finalization sequence to delete the remote lane, rename the local temp lane, and export atomically
- Enhanced the
archiveLanemethod to support throwing errors for critical operations and gracefully handle concurrent deletions - Added comprehensive e2e tests to verify the temp lane mechanism works correctly
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scopes/git/ci/ci.main.runtime.ts | Implements temporary lane strategy with random suffixes, updates finalization logic to minimize race window, and improves error handling in lane deletion |
| e2e/harmony/ci-commands.e2e.ts | Adds test coverage for temp lane creation, renaming, and cleanup to verify race condition mitigation works as intended |
GiladShoham
approved these changes
Feb 4, 2026
Comment on lines
+468
to
+474
| this.logger.console(chalk.blue(`Cleaning up temporary lane ${tempLaneFullName}`)); | ||
| try { | ||
| await this.lanes.removeLanes([tempLaneFullName], { remote: false, force: true }); | ||
| this.logger.console(chalk.green(`Removed temporary lane ${tempLaneFullName}`)); | ||
| } catch (cleanupErr: any) { | ||
| // Ignore cleanup errors to avoid masking the original error | ||
| this.logger.console(chalk.yellow(`Failed to clean up temporary lane: ${cleanupErr?.message || cleanupErr}`)); |
There was a problem hiding this comment.
The error cleanup logic doesn't account for the case where the temp lane has already been renamed to the final name. If the rename operation (line 439) succeeds but the export (line 443) fails, the lane will have the final name (laneId.name), not the temp name. The cleanup code should check whether the rename completed and clean up the appropriately named lane. Consider tracking the rename status in a variable and using it to determine which lane name to clean up.
Suggested change
| this.logger.console(chalk.blue(`Cleaning up temporary lane ${tempLaneFullName}`)); | |
| try { | |
| await this.lanes.removeLanes([tempLaneFullName], { remote: false, force: true }); | |
| this.logger.console(chalk.green(`Removed temporary lane ${tempLaneFullName}`)); | |
| } catch (cleanupErr: any) { | |
| // Ignore cleanup errors to avoid masking the original error | |
| this.logger.console(chalk.yellow(`Failed to clean up temporary lane: ${cleanupErr?.message || cleanupErr}`)); | |
| const finalLaneFullName = `${laneId.scope}/${laneId.name}`; | |
| this.logger.console( | |
| chalk.blue( | |
| `Cleaning up CI lane. Trying temporary lane ${tempLaneFullName}${ | |
| tempLaneFullName !== finalLaneFullName ? ` or final lane ${finalLaneFullName}` : '' | |
| }` | |
| ) | |
| ); | |
| try { | |
| await this.lanes.removeLanes([tempLaneFullName], { remote: false, force: true }); | |
| this.logger.console(chalk.green(`Removed temporary lane ${tempLaneFullName}`)); | |
| } catch (cleanupErr: any) { | |
| this.logger.console( | |
| chalk.yellow( | |
| `Failed to clean up temporary lane ${tempLaneFullName}: ${cleanupErr?.message || cleanupErr}` | |
| ) | |
| ); | |
| // If the lane was renamed before the failure, it may now exist under the final name. | |
| if (tempLaneFullName !== finalLaneFullName) { | |
| try { | |
| await this.lanes.removeLanes([finalLaneFullName], { remote: false, force: true }); | |
| this.logger.console(chalk.green(`Removed CI lane under final name ${finalLaneFullName}`)); | |
| } catch (finalCleanupErr: any) { | |
| // Ignore cleanup errors to avoid masking the original error | |
| this.logger.console( | |
| chalk.yellow( | |
| `Failed to clean up CI lane under final name ${finalLaneFullName}: ${ | |
| finalCleanupErr?.message || finalCleanupErr | |
| }` | |
| ) | |
| ); | |
| } | |
| } |
| const tempLaneFullName = `${laneId.scope}/${tempLaneName}`; | ||
| this.logger.console(chalk.blue(`Cleaning up temporary lane ${tempLaneFullName}`)); | ||
| try { | ||
| await this.lanes.removeLanes([tempLaneFullName], { remote: false, force: true }); |
There was a problem hiding this comment.
The temp lane cleanup uses the wrong format for the lane name. When removing a local lane (remote: false), the lane name should not include the scope prefix. This should be just tempLaneName instead of tempLaneFullName. The current code may fail to clean up the temporary lane on error, leaving orphaned lanes in the workspace.
Suggested change
| await this.lanes.removeLanes([tempLaneFullName], { remote: false, force: true }); | |
| await this.lanes.removeLanes([tempLaneName], { remote: false, force: true }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When multiple CI jobs run
bit ci pron the same branch concurrently, a race condition could cause one job to fail with "lane exists with different hash".The problem:
The fix:
Use a temporary lane name with a random suffix (e.g.,
pr-branch-a1b2c) during the snap operation, then rename to the final name right before export. This minimizes the race window to just the quick delete-rename-export operations.Changes:
snapPrCommitto create temp lane, snap, then delete/rename/exportFresh branch to rule out CI caching issues from #10172