fix: rollback failed notification interaction with visual warning

[setchy]

Issue
Fix a bug whereby a notification interaction can fail [rate limit, forbidden] leaving gitify in a broken state. manual or timer notification refreshes would not recover state, only a force reload of the app.

Example
Using a GitHub Enterprise Managed User (EMU) account, that has subscribed to a GitHub Cloud (OSS) issue or pull request.
The GitHub EMU security policies prevent API interactions via PAT with GitHub Cloud content.
This throws a HTTP 403 Forbidden when performing mark as read, mark as done or unsubscribe interactions.

Before

before.mov

After
Notification state restored on failure [tanstack query]
Visual indicator on notification interaction buttons about reason for failure

Screen.Recording.2026-08-04.at.8.08.04.AM.mov

[sonarqubecloud]

Quality Gate failed

Failed conditions
3.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

[setchy]

@codebytere - this may help with some of the occasional random state corruption you've previously reported

[afonsojramos]

If the follow-up mark-as-done/mark-as-read fails, this still returns result - which is { succeeded: [notification], failed: [] } - so the onSuccess below iterates succeeded and calls clearFailure(notification.id), wiping the failure the inner mutation just recorded.

The notification stays in the list (the rollback itself works), but there is no red button and no tooltip. Worse, NotificationRow.runAction then sees no failure, so it never reverts shouldAnimateNotificationExit. The row sits at translate-x-full opacity-0 with its hover actions hidden. That is exactly the ghost-row state this PR sets out to fix.

Reproduced against this branch: unsubscribe resolves, markThreadAsRead 403s, notificationCount is 1 as expected, but notificationFailures[id] is undefined.

Returning the composed result fixes it:

const followUp = markAsDoneOnUnsubscribe
  ? await markNotificationsAsDoneMutation.mutateAsync({ doneNotifications: [notification] })
  : await markNotificationsAsReadMutation.mutateAsync({ readNotifications: [notification] });

return followUp.failed.length > 0 ? followUp : result;

[afonsojramos]

This builds the result by filtering snapshotEntry.notifications, so anything present in current but absent from snapshot is silently dropped. Reachable whenever a poll lands during a bulk action: refetchInterval is not blocked by the mutation, and the cancelQueries in onMutate only cancels what was already in flight at that moment.

Reproduced: snapshot [first, second], current [first, second, third], fail second → result is [first, second]. third disappears from the list until the next poll.

Starting from current and adding back only what is missing also makes the "normally a no-op" note above actually true — today this rewrites the account's array on every failure:

const currentIds = new Set(accountEntry.notifications.map((n) => n.id));
const missing = snapshotEntry.notifications.filter(
  (n) => failedIds.has(n.id) && !currentIds.has(n.id),
);

if (missing.length === 0) {
  return accountEntry;
}

return { ...accountEntry, notifications: [...accountEntry.notifications, ...missing] };

Worth a mutations.test.ts case where current holds a notification the snapshot does not.

[afonsojramos]

HoverButton passes label into both title and aria-label, so when any one action fails, all three buttons here (166, 175, 184) announce "Action Forbidden: GitHub rejected this action…" and all three turn red (168, 177, 186).

Two problems: a screen reader user can no longer tell the three buttons apart, and the red signals failure for two actions that were never attempted.

failure.action already records which action failed — but nothing in the PR reads it, so it is write-only today. Gating on it fixes both halves:

label="Mark as read"
variant={failure?.action === 'markAsRead' ? 'danger' : 'invisible'}

At minimum, keep aria-label as the action name and put the failure text in title only.

[afonsojramos]

Keying by bare notification.id collides across accounts. IDs are only unique per instance: gitea/transform.ts:31 is String(raw.id) (small sequential ints), bitbucket uses head.notificationId, and a fresh GHES restarts its own sequence. Two Gitea instances, or Gitea + GHES, collide readily — a failure on account A then paints the row for account B, and pruneFailures can clear the wrong entry.

The codebase already keys by account for exactly this reason: notifications.ts:216 uses ${getAccountUUID(account)}:${id}:${updatedAt}. Suggest the same composite key here.

[afonsojramos]

When !isMarkAsDoneFeatureSupported (Gitea, GHES < 3.13 — both reachable), the read mutation's onSuccess has already run reconcileFailedNotifications(..., 'markAsRead', ...). This runs it a second time with 'markAsDone', overwriting the entry and emitting a duplicate rendererLogError per notification.

Worth skipping in the fallback path the same way the cache write just above it is skipped.

[afonsojramos]

This makes any unclassified 403 an ACTION_FORBIDDEN, but determineFailureType is also called from notifications.ts:104 (the list fetch) and Accounts.tsx:87 (account refresh). A 403 on the notifications list (SAML/SSO enforcement, a suspended org) will now render the full-screen error "Action Forbidden — GitHub rejected this action for this account when performed via Gitify", which reads oddly for a read that involved no action.

Maybe a more neutral wording?

[afonsojramos]

Tested this rollback behaviour end-to-end. I think that the direction is right, and settleNotificationActions is a real improvement on its own over the old Promise.all, which meant a single 403 discarded the outcome of every other notification in a bulk action.

I did reproduce two bugs with throwaway tests on this branch, plus an accessibility regression, are inline below.

Smaller items, not worth their own threads:

• cancelQueries in onMutate with no optimistic update. All three handlers await queryClient.cancelQueries({ queryKey: notificationsKeys.all }), but the cache write deliberately happens in onSuccess so the exit animation has time to play. That leaves the defensive restore as the snapshot's only consumer. Cancelling an in-flight poll on every click buys little. May be worth snapshotting without cancelling, or a note on why the cancel is wanted.
• Every NotificationRow subscribes to the whole failures map. useNotifications returns useNotificationActionFailuresStore((s) => s.failures), so a single setFailure re-renders every mounted row. Fine at typical inbox sizes, but a per-id selector would avoid it.
• Nit: reconcileFailedNotifications re-declares its failed param inline as Array<{ notification; error; rawError }> when FailedNotificationAction[] is already exported from mutations.ts.

CI: the SonarCloud quality gate is red: 3.4% duplication on new code against a 3% gate. The three identical onMutate blocks are the obvious candidate; hoisting them into a shared snapshotNotifications callback should clear it.

I rebased the branch onto main and pushed, the only conflict was an import block in useNotifications.ts (main's Constants import vs this PR's extended stores import).