Skip to content

Commit 65dfddc

Browse files
authored
fix(vscode): dont show CIPE notification multiple times even after ai fixes timed out (#2739)
1 parent a3a2428 commit 65dfddc

File tree

2 files changed

+81
-8
lines changed

2 files changed

+81
-8
lines changed

libs/vscode/nx-cloud-view/src/cipe-notifications.spec.ts

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -706,20 +706,31 @@ describe('CIPE Notifications', () => {
706706
);
707707
});
708708

709-
it('should show regular error notification if an existing run fails and there is no AI fix after 5 minutes', () => {
709+
it('should show regular error notification when transitioning to no AI fix after 5 minutes', () => {
710710
const sixMinutesAgo = Date.now() - 1000 * 60 * 6;
711+
712+
// CIPE that was waiting for AI fix (within 5 minutes)
713+
const waitingCipe: CIPEInfo[] = [
714+
{
715+
...pipelineExamples.failWithAiFixesEnabled[0],
716+
createdAt: tenMinutesAgo,
717+
completedAt: oneMinuteAgo, // Within 5 minutes
718+
},
719+
];
720+
721+
// Same CIPE but now past 5 minutes
711722
const failedCipe: CIPEInfo[] = [
712723
{
713724
...pipelineExamples.failWithAiFixesEnabled[0],
714725
createdAt: tenMinutesAgo,
715-
completedAt: sixMinutesAgo,
726+
completedAt: sixMinutesAgo, // Past 5 minutes
716727
},
717728
];
718729

719730
jest.clearAllMocks();
720731

721-
// should also fail if same CIPE is re-checked again but timeout has passed
722-
compareCIPEDataAndSendNotification(failedCipe, failedCipe);
732+
// Transition from waiting to timeout - should show delayed notification
733+
compareCIPEDataAndSendNotification(waitingCipe, failedCipe);
723734
expect(window.showInformationMessage).not.toHaveBeenCalled();
724735
expect(window.showErrorMessage).toHaveBeenCalledTimes(1);
725736
expect(window.showErrorMessage).toHaveBeenCalledWith(
@@ -729,6 +740,55 @@ describe('CIPE Notifications', () => {
729740
'View Results',
730741
);
731742
});
743+
744+
it('should not show delayed notification repeatedly - only once when transitioning from waiting to no AI fix', () => {
745+
const sixMinutesAgo = Date.now() - 1000 * 60 * 6;
746+
747+
// CIPE that previously could have had AI fix (within 5 minutes)
748+
const cipeWaitingForAiFix: CIPEInfo[] = [
749+
{
750+
...pipelineExamples.failWithAiFixesEnabled[0],
751+
createdAt: tenMinutesAgo,
752+
completedAt: oneMinuteAgo, // Within 5 minutes, so still waiting
753+
},
754+
];
755+
756+
// Same CIPE but now past 5 minutes
757+
const cipeAfterTimeout: CIPEInfo[] = [
758+
{
759+
...pipelineExamples.failWithAiFixesEnabled[0],
760+
createdAt: tenMinutesAgo,
761+
completedAt: sixMinutesAgo, // Past 5 minutes, no AI fix coming
762+
},
763+
];
764+
765+
// First transition: from waiting to timeout - should show delayed notification
766+
compareCIPEDataAndSendNotification(
767+
cipeWaitingForAiFix,
768+
cipeAfterTimeout,
769+
);
770+
expect(window.showErrorMessage).toHaveBeenCalledTimes(1);
771+
expect(window.showErrorMessage).toHaveBeenCalledWith(
772+
'CI failed for #feature.',
773+
'Help me fix this error',
774+
'View Commit',
775+
'View Results',
776+
);
777+
778+
jest.clearAllMocks();
779+
780+
// Subsequent checks with same state - should NOT show notification again
781+
compareCIPEDataAndSendNotification(cipeAfterTimeout, cipeAfterTimeout);
782+
expect(window.showErrorMessage).not.toHaveBeenCalled();
783+
expect(window.showInformationMessage).not.toHaveBeenCalled();
784+
785+
jest.clearAllMocks();
786+
787+
// Another check - should still NOT show notification
788+
compareCIPEDataAndSendNotification(cipeAfterTimeout, cipeAfterTimeout);
789+
expect(window.showErrorMessage).not.toHaveBeenCalled();
790+
expect(window.showInformationMessage).not.toHaveBeenCalled();
791+
});
732792
});
733793

734794
it('should not show regular notifications twice even if run has been failed for more than 5 minutes and ai fixes are not enabled', () => {

libs/vscode/nx-cloud-view/src/cipe-notifications.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,25 @@ export function compareCIPEDataAndSendNotification(
8888
// if the cipe has completed somehow or had a failed run before the latest update,
8989
// we've already shown a notification and can return
9090
// the one exception is if we supressed the notification because we thought an AI fix might be coming
91-
// if ai fixes aren't enabled, we never do this suppression
92-
if (
91+
// and now we know no AI fix is coming after 5 minutes AND we haven't shown this delayed notification yet
92+
const oldPotentiallyHasAiFix =
93+
(oldCIPE?.runGroups || []).some((runGroup) => !!runGroup.aiFix) ||
94+
(oldCIPE?.aiFixesEnabled &&
95+
oldCIPE.status === 'FAILED' &&
96+
oldCIPE.completedAt &&
97+
oldCIPE.completedAt + 1000 * 60 * 5 >= Date.now());
98+
99+
const isDelayedNotificationTransition =
100+
oldPotentiallyHasAiFix &&
101+
!potentiallyHasAiFix &&
102+
failedButNoAiFixInFiveMinutes;
103+
104+
const shouldSkipDueToCompletedCIPE =
93105
oldCIPE &&
94106
(oldCIPE.status !== 'IN_PROGRESS' || oldCIPEFailedRun) &&
95-
(!failedButNoAiFixInFiveMinutes || !newCIPE.aiFixesEnabled)
96-
) {
107+
!isDelayedNotificationTransition;
108+
109+
if (shouldSkipDueToCompletedCIPE) {
97110
return;
98111
}
99112

0 commit comments

Comments
 (0)