feat: ETag conditional requests for version checks

[TerrifiedBug]

Summary

• Add ETag support to GitHub API release checks — 304 Not Modified responses don't count against rate limits
• Reduce check interval from 24 hours to 15 minutes
• Store ETags per endpoint in SystemSettings (3 new nullable fields)
• Fully backward compatible — first request after upgrade has no ETag, behaves like before

Test Plan

• Deploy and verify version check still works on Settings page
• Check DB after first check — latestServerReleaseEtag should be populated
• Wait 15 min, trigger another check — verify it uses cached data on 304
• Force check — verify it fetches fresh data

[greptile-apps]

Greptile Summary

This PR adds ETag-based conditional request support to the three GitHub API version-check functions and reduces the polling interval from 24 hours to 15 minutes, leveraging the fact that 304 responses don't count against GitHub's rate limit. The schema and migration changes are clean and backward-compatible.

Key findings:

• Logic bug — force flag doesn't actually force a fresh fetch: All three check functions pass the stored ETag even when force = true. GitHub can still respond with 304, and the caller receives cached data. The PR's own test plan item ("Force check — verify it fetches fresh data") would fail. The ETag should be set to null when force is true.
• Logic issue — misleading checkedAt on API failure: checkedAt is unconditionally set to new Date() before the API response is evaluated. On a silent API failure (no 304, no 200), nothing is written to the DB, but the function still returns checkedAt = new Date() — telling the UI the check happened just now when it didn't. The old code only set checkedAt inside the success branches. Same issue on lines 173 and 250.

Confidence Score: 3/5

• Unsafe to merge as-is: the force-check ETag bypass means forced refreshes can silently return stale data, which is a meaningful functional regression.
• The overall ETag implementation is sound and the migration is backward-compatible. However, the logic bug where force = true still sends If-None-Match means forced refreshes can silently return stale data, which is a meaningful functional regression for anyone using the force-check feature. The misleading checkedAt on API failure is a secondary issue that affects UI accuracy. Both issues require fixes before merge.
• src/server/services/version-check.ts — specifically the ETag passthrough on all three forced-check call sites (lines 99–102, 169–172, 246–249) and checkedAt assignment logic (lines 103, 173, 250).

_{Last reviewed commit: 108e148}

[greptile-apps]

force flag does not bypass ETag — forced checks can still return cached data

When force = true, needsCheck is forced to true, but the stored ETag is still passed to fetchRelease. This means GitHub can still respond with 304 Not Modified, and the function returns the previously-cached version instead of freshly fetched data.

This directly contradicts the expected behaviour described in the PR's own test plan: "Force check — verify it fetches fresh data." A user hitting "Force Check" on the Settings page would silently receive stale data if nothing changed on GitHub's side.

The same problem exists in checkAgentVersion (lines 169–172) and checkDevAgentVersion (lines 246–249).

The fix is to pass null for the ETag when force is true:

Suggested change

	const { release, etag, notModified } = await fetchRelease(
	`${GITHUB_API}/repos/${SERVER_REPO}/releases/latest`,
	settings?.latestServerReleaseEtag,
	);
	const { release, etag, notModified } = await fetchRelease(
	`${GITHUB_API}/repos/${SERVER_REPO}/releases/latest`,
	force ? null : settings?.latestServerReleaseEtag,
	);

Apply the same pattern in the other two functions.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 99-102

Comment:
`force` flag does not bypass ETag — forced checks can still return cached data

When `force = true`, `needsCheck` is forced to `true`, but the stored ETag is still passed to `fetchRelease`. This means GitHub can still respond with `304 Not Modified`, and the function returns the previously-cached version instead of freshly fetched data.

This directly contradicts the expected behaviour described in the PR's own test plan: _"Force check — verify it fetches fresh data."_ A user hitting "Force Check" on the Settings page would silently receive stale data if nothing changed on GitHub's side.

The same problem exists in `checkAgentVersion` (lines 169–172) and `checkDevAgentVersion` (lines 246–249).

The fix is to pass `null` for the ETag when `force` is `true`:

```suggestion
const { release, etag, notModified } = await fetchRelease(
  `${GITHUB_API}/repos/${SERVER_REPO}/releases/latest`,
  force ? null : settings?.latestServerReleaseEtag,
);
```

Apply the same pattern in the other two functions.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Misleading checkedAt on API failure

checkedAt is unconditionally set to new Date() before the API response is evaluated. If the API returns an error (not 304, not 200), neither the notModified nor the release branch fires, so nothing is written to the DB. The next call will correctly retry (the DB timestamp was never bumped), but the function still returns checkedAt = new Date(), telling the UI "last checked: just now" when the check actually failed silently.

In the old code, checkedAt = new Date() was set only inside the success branches, so on failure the UI would show the last successful check time instead.

The same pattern exists on lines 173 and 250 in checkAgentVersion and checkDevAgentVersion.

Consider only setting checkedAt = new Date() inside the branches that actually persist to the DB, or tracking a separate checkFailed state.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/server/services/version-check.ts
Line: 103

Comment:
Misleading `checkedAt` on API failure

`checkedAt` is unconditionally set to `new Date()` before the API response is evaluated. If the API returns an error (not 304, not 200), neither the `notModified` nor the `release` branch fires, so nothing is written to the DB. The next call will correctly retry (the DB timestamp was never bumped), but the function still *returns* `checkedAt = new Date()`, telling the UI "last checked: just now" when the check actually failed silently.

In the old code, `checkedAt = new Date()` was set only inside the success branches, so on failure the UI would show the last successful check time instead.

The same pattern exists on lines 173 and 250 in `checkAgentVersion` and `checkDevAgentVersion`.

Consider only setting `checkedAt = new Date()` inside the branches that actually persist to the DB, or tracking a separate `checkFailed` state.

How can I resolve this? If you propose a fix, please make it concise.