perf(notifications): use conditional requests for the notifications list

[joshfree]

Fixes part of #3101 (Improvement 2 of 3).

Problem

The notifications list is requested with caching disabled and no validators
(listNotificationsForAuthenticatedUser in client.ts sets
Cache-Control: no-cache), so the full list body is transferred and re-parsed
on every poll even when nothing has changed.

Change

Cache the previous notifications-list response per account and send conditional
request validators (If-None-Match from the stored ETag, falling back to
If-Modified-Since from Last-Modified). When GitHub responds 304 Not Modified, the cached list is returned without transferring or re-parsing the
body. A 304 also does not count against the primary rate limit.

Notes:

• Octokit throws a RequestError with status === 304 for an unchanged
  conditional request, so that is caught and mapped back to the cached list.
• Cache-Control: no-cache is retained to force revalidation rather than serve
  a stale locally-cached response; the validators are added alongside it.
• The paginated ("fetch all") path captures the validators from the first page.

Tests

Added coverage in client.test.ts for: replaying the stored ETag as
If-None-Match on the next poll, and returning the cached list on a 304 for
both the single-page and paginated paths.

[setchy]

i have a feeling the explicit no-cache on this api route was due to this user-flow

#3102 (comment)
There was also some interesting notification timestamp behavior as notifications go from your unread inbox, to done/read, to being manually moved back to your inbox - so we'd need to go ensure our cache key handles this, too

we'll need to test how this PR handles situations like this

[afonsojramos]

I dug into this one empirically, because the "quirky behavior" I half-remembered turned out to be real, measurable, and unfortunately fatal for the ETag half of this approach.

The notifications endpoints return a degenerate ETag. Measured with the exact headers Octokit sends (Accept: application/vnd.github+json, gzip):

• Whenever the list is non-empty, GET /notifications returns ETag: W/"" (an empty opaque value). Other endpoints return real hashes (/user gives W/"3233...").
• Replaying If-None-Match: W/"" returns 304 Not Modified even after the list has demonstrably changed (new notifications arrived, Last-Modified advanced). Since W/"" is constant, the conditional always matches.
• The ETag only becomes a real hash when the list is empty, so it only detects transitions through empty.

Since this PR prefers the stored ETag over Last-Modified, the first poll stores W/"" and every subsequent poll gets 304: the client would never see a new notification again until the list passes through empty or the app restarts. The unit tests pass because the mocks use a realistic W/"abc", which the real endpoint never returns.

Last-Modified/If-Modified-Since works, but with a blind spot. I drove a disposable notification thread (scratch repo, github-actions[bot] as the second actor) through its full lifecycle and measured at each step:

Event	list body	Last-Modified	conditional result
New comment / state change	changes	bumps	200 ✓
Label added	unchanged	unchanged	304
Mark thread read (PATCH)	changes (thread leaves)	unchanged	304 (stale)
Mark thread done (DELETE)	changes (thread leaves)	unchanged	304 (stale)

Last-Modified is just max(updated_at) of the listed threads (it even moves backwards when the newest thread leaves the list). Read-state changes never bump it. Consequences for an If-Modified-Since version of this PR:

1. After Gitify itself marks a thread read/done, the next poll gets 304 and would serve the stale cached list that still contains the thread, resurrecting it in the UI. The cache would need to be invalidated in the mark-read/done/unsubscribe mutation paths.
2. Read-state changes made on github.com or another device stay invisible until some real activity bumps Last-Modified. Today's unconditional polling picks those up within one interval, so this is a cross-device sync regression. Window-focus and manual refresh don't self-heal either, since they flow through the same conditional request path.

Also verified while testing: 304s genuinely don't count against the primary rate limit, so the premise is right. But the steady-state saving here is roughly 60 requests/hour/account against the 5,000/hour budget, and with #3102 the recurring enrichment volume (the bulk of #3101) already collapses.

Given that, I don't think this is worth the correctness surface as-is. I've opened #3116 implementing improvement 3 (X-Poll-Interval), which captures the polite-polling win with zero staleness risk. If we still want conditional requests afterwards, I think the requirements are: If-Modified-Since only (discard W/"" ETags), cache invalidation on our own mutations, an unconditional fetch on manual refresh/window focus, and an explicit call on the cross-device tradeoff. Happy to share the probe workflow for reproducing any of the measurements above.

[joshfree]

Really appreciate the depth here — the degenerate W/"" ETag on /notifications is a great catch, and you're right that it breaks the ETag path (and that my tests only passed because the mocks used realistic ETags the real endpoint never returns). Combined with the Last-Modified read-state blind spot, and the fact that #3102 already captures the bulk of the win, I agree this isn't worth the correctness surface. Closing in favor of #3116 for the polite-polling piece. Thanks for the thorough measurements!