From a7d340def550c9046ed3d82456f01c26077f2abd Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Wed, 3 Jun 2026 22:57:16 +0200 Subject: [PATCH 1/2] fix(filters): keep directly-addressed notifications when excluding by author --- .../utils/notifications/filters/filter.ts | 13 ++++++++- .../utils/notifications/filters/search.ts | 27 +++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/renderer/utils/notifications/filters/filter.ts b/src/renderer/utils/notifications/filters/filter.ts index eb9ebdda6..7a0d78e27 100644 --- a/src/renderer/utils/notifications/filters/filter.ts +++ b/src/renderer/utils/notifications/filters/filter.ts @@ -8,11 +8,13 @@ import type { } from '../../../types'; import { + AUTHOR_SEARCH_PREFIX, BASE_SEARCH_QUALIFIERS, DETAILED_ONLY_SEARCH_QUALIFIERS, filterNotificationBySearchTerm, hasExcludeSearchFilters, hasIncludeSearchFilters, + isDirectlyAddressedReason, reasonFilter, type SearchQualifier, stateFilter, @@ -116,7 +118,16 @@ function passesSearchTokenFiltersForQualifier( if (hasExcludeSearchFilters()) { const excludeTokens = filters.excludeSearchTokens.filter((t) => t.startsWith(prefix)); - if (excludeTokens.length > 0) { + + // An `author:` exclude must not hide notifications that directly address + // the user (e.g. review requested), since `subject.user` may be a bot that + // only left the latest comment rather than the actor who addressed them. + // `reason` is optional-chained because `isUserFilteredOut` builds synthetic + // notifications without one. + const skipExclude = + prefix === AUTHOR_SEARCH_PREFIX && isDirectlyAddressedReason(notification.reason?.code); + + if (excludeTokens.length > 0 && !skipExclude) { passes = passes && !excludeTokens.some((token) => filterNotificationBySearchTerm(notification, token)); diff --git a/src/renderer/utils/notifications/filters/search.ts b/src/renderer/utils/notifications/filters/search.ts index e23df7018..b3a731832 100644 --- a/src/renderer/utils/notifications/filters/search.ts +++ b/src/renderer/utils/notifications/filters/search.ts @@ -1,12 +1,35 @@ import { useFiltersStore } from '../../../stores'; -import type { RawGitifyNotification } from '../../../types'; +import type { RawGitifyNotification, Reason } from '../../../types'; export const SEARCH_DELIMITER = ':'; +export const AUTHOR_SEARCH_PREFIX = 'author:'; + +/** + * Notification reasons where the user was directly asked to act (review, + * approval, assignment, mention). An `author:` exclude filter must not hide + * these: after enrichment a notification's `subject.user` can resolve to a bot + * that merely left the most recent comment rather than the actor who addressed + * the user, so excluding that bot would drop actionable notifications. + * + * See https://github.com/gitify-app/gitify/issues/2914. + */ +const DIRECTLY_ADDRESSED_REASONS: ReadonlySet = new Set([ + 'approval_requested', + 'assign', + 'mention', + 'review_requested', + 'team_mention', +]); + +export function isDirectlyAddressedReason(reason: Reason | undefined): boolean { + return reason !== undefined && DIRECTLY_ADDRESSED_REASONS.has(reason); +} + const SEARCH_QUALIFIERS = { author: { - prefix: 'author:', + prefix: AUTHOR_SEARCH_PREFIX, description: 'filter by notification author', requiresDetailsNotifications: true, extract: (n: RawGitifyNotification) => n.subject?.user?.login, From a3fb838bfc68ab920ef10f1b65af107f0d49199d Mon Sep 17 00:00:00 2001 From: Afonso Jorge Ramos Date: Wed, 3 Jun 2026 22:57:31 +0200 Subject: [PATCH 2/2] test(filters): cover author exclude exemption for directly-addressed reasons --- .../notifications/filters/filter.test.ts | 66 +++++++++++++++++++ .../notifications/filters/search.test.ts | 27 +++++++- 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/src/renderer/utils/notifications/filters/filter.test.ts b/src/renderer/utils/notifications/filters/filter.test.ts index ebf44cecd..6f3360622 100644 --- a/src/renderer/utils/notifications/filters/filter.test.ts +++ b/src/renderer/utils/notifications/filters/filter.test.ts @@ -182,6 +182,72 @@ describe('renderer/utils/notifications/filters/filter.ts', () => { expect(result).toEqual([mockNotifications[0]]); }); + it('should not exclude directly-addressed notifications by author handle (#2914)', async () => { + // A human-authored PR where a bot left the latest comment, so + // enrichment resolves `subject.user` to the bot, but the user was + // directly asked to review. + const reviewRequested = mockPartialGitifyNotification({ + title: 'Please review this PR', + user: { + login: 'coderabbitai', + htmlUrl: 'https://github.com/coderabbitai' as Link, + avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link, + type: 'Bot', + }, + }); + reviewRequested.reason.code = 'review_requested'; + + // A notification that exists only because the bot commented should + // still be excluded. + const botComment = mockPartialGitifyNotification({ + title: 'Bot comment noise', + user: { + login: 'coderabbitai', + htmlUrl: 'https://github.com/coderabbitai' as Link, + avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link, + type: 'Bot', + }, + }); + botComment.reason.code = 'subscribed'; + + useFiltersStore.setState({ + excludeSearchTokens: ['author:coderabbitai' as SearchToken], + }); + + const result = filterDetailedNotifications([reviewRequested, botComment], { + ...mockSettings, + detailedNotifications: true, + }); + + expect(result).toEqual([reviewRequested]); + }); + + it('should still apply author include filters for directly-addressed reasons', async () => { + const reviewRequested = mockPartialGitifyNotification({ + title: 'Please review this PR', + user: { + login: 'coderabbitai', + htmlUrl: 'https://github.com/coderabbitai' as Link, + avatarUrl: 'https://avatars.githubusercontent.com/u/1' as Link, + type: 'Bot', + }, + }); + reviewRequested.reason.code = 'review_requested'; + + // The exemption applies to excludes only; an author include still + // narrows to matching notifications. + useFiltersStore.setState({ + includeSearchTokens: ['author:someone-else' as SearchToken], + }); + + const result = filterDetailedNotifications([reviewRequested], { + ...mockSettings, + detailedNotifications: true, + }); + + expect(result).toEqual([]); + }); + it('should filter notifications by state when provided', async () => { useFiltersStore.setState({ states: ['closed'] }); diff --git a/src/renderer/utils/notifications/filters/search.test.ts b/src/renderer/utils/notifications/filters/search.test.ts index b3778f8a7..e9e456807 100644 --- a/src/renderer/utils/notifications/filters/search.test.ts +++ b/src/renderer/utils/notifications/filters/search.test.ts @@ -2,7 +2,12 @@ import { mockPartialGitifyNotification } from '../../../__mocks__/notifications- import type { GitifyOwner, Link } from '../../../types'; -import { ALL_SEARCH_QUALIFIERS, filterNotificationBySearchTerm, parseSearchInput } from './search'; +import { + ALL_SEARCH_QUALIFIERS, + filterNotificationBySearchTerm, + isDirectlyAddressedReason, + parseSearchInput, +} from './search'; // (helper removed – no longer used) @@ -89,4 +94,24 @@ describe('renderer/utils/notifications/filters/search.ts', () => { expect(filterNotificationBySearchTerm(mockNotification, '')).toBe(false); }); }); + + describe('isDirectlyAddressedReason', () => { + it('returns true for reasons that ask the user to act', () => { + expect(isDirectlyAddressedReason('approval_requested')).toBe(true); + expect(isDirectlyAddressedReason('assign')).toBe(true); + expect(isDirectlyAddressedReason('mention')).toBe(true); + expect(isDirectlyAddressedReason('review_requested')).toBe(true); + expect(isDirectlyAddressedReason('team_mention')).toBe(true); + }); + + it('returns false for passive reasons', () => { + expect(isDirectlyAddressedReason('subscribed')).toBe(false); + expect(isDirectlyAddressedReason('comment')).toBe(false); + expect(isDirectlyAddressedReason('author')).toBe(false); + }); + + it('returns false for undefined reason', () => { + expect(isDirectlyAddressedReason(undefined)).toBe(false); + }); + }); });