Skip to content

app(sync): keep backendBusy cooldown in-memory only#7527

Merged
mdmohsin7 merged 2 commits into
mainfrom
caleb/sync-busy-no-persist
Jun 1, 2026
Merged

app(sync): keep backendBusy cooldown in-memory only#7527
mdmohsin7 merged 2 commits into
mainfrom
caleb/sync-busy-no-persist

Conversation

@mdmohsin7
Copy link
Copy Markdown
Member

Summary

  • SyncRateLimiter now persists only the rateLimit cooldown. backendBusy cooldowns live in an in-memory field that clears on app restart.
  • Constructor clears any pre-existing persisted backendBusy entry from older app versions, so users coming from a healthy server get unstuck on next launch.

Why

The "Omi servers are busy" banner was surviving app restarts even when the backend had long since recovered. The cooldown was saved to SharedPreferences for both rateLimit (HTTP 429) and backendBusy (stale-guard) cases.

rateLimit cooldowns SHOULD persist — they mirror server-side fair-use enforcement that is itself sticky across restarts (30-day restrict window). But backendBusy reflects transient backend pipeline pressure. If a user restarts the app to retry, the cooldown should clear and the next sync should re-probe the backend, not trust a local timer set during an earlier rough patch.

Backend data (last 7 days): zero stale-guard fires globally, yet some users still report seeing the banner. Their app state is stuck from an earlier server hiccup that has long since resolved.

Behavior

State Before After
Server returns 429 (rateLimit) 30 min cooldown, persists across restart unchanged — still persists
Reconciler sees stale-guard (backendBusy) 10 min cooldown, persists across restart 10 min cooldown, in-memory only
App restart while in backendBusy banner persists until 10 min wall-clock elapses banner clears immediately, next sync re-probes
User upgrades from old version with stuck backendBusy banner persists one-time migration clears it on first launch

Test plan

  • Manual: mark backendBusy via debug hook (or hit the trigger condition), kill app, relaunch — banner gone
  • Manual: mark rateLimit (e.g. fair-use 429), kill app, relaunch — banner still there, cooldown still ticking
  • Manual: install build over an older one that had persisted backendBusy state — banner gone after launch
  • No regression in successful-sync flow (banner clears on success)

The "Omi servers are busy" message persisted across app restarts because
the SyncRateLimiter saved its until/reason to SharedPreferences for both
rate-limit (HTTP 429) and backendBusy (stale-guard) cases.

backendBusy reflects transient backend pipeline pressure. If a user
restarts the app to retry, the cooldown should clear and the next sync
should re-probe the backend instead of trusting a local timer set
during an earlier rough patch.

Keep persistence for rateLimit (mirrors server-side fair-use enforcement
which IS sticky across restarts) and move backendBusy to an in-memory
field. On construction, clear any pre-existing persisted backendBusy
entry so users coming from older app versions get unstuck on next
launch.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR fixes a UX regression where the "Omi servers are busy" banner persisted across app restarts because backendBusy cooldowns were stored in SharedPreferences alongside rateLimit cooldowns. The fix moves backendBusy state to an in-memory field (_backendBusyUntilMs) that clears on restart, while keeping rateLimit cooldowns persisted as before.

  • SyncRateLimiter now routes markLimited calls through a branch: backendBusy sets _backendBusyUntilMs, while rateLimit writes to SharedPreferences as before.
  • A one-time constructor migration clears any pre-existing persisted backendBusy entry so users upgrading from older builds get unstuck immediately.
  • The reason getter's consistency with until is fixed: both now return values belonging to whichever cooldown expires later (max-based alignment), resolving the mismatch flagged in the previous review thread.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to a single service class and correctly separates transient from durable cooldown state.

The backendBusy/rateLimit split is implemented correctly across all getter/setter paths. The max-based alignment between until and reason — the concern raised in the prior review thread — is properly resolved. The one-time migration cleanly handles users upgrading from older builds. No data-loss or incorrect-state scenarios were found.

No files require special attention.

Important Files Changed

Filename Overview
app/lib/services/wals/sync_rate_limiter.dart Splits cooldown persistence: backendBusy is now in-memory only via _backendBusyUntilMs; rateLimit still persists to SharedPreferences. Adds a one-time migration that clears stuck backendBusy prefs entries on first launch. The reason/until consistency issue flagged in the previous review is addressed by aligning both getters to a max-based pick of the longer-lived deadline.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[markLimited called] --> B{reason?}
    B -- backendBusy --> C[_backendBusyUntilMs = now + secs]
    B -- rateLimit --> D[SharedPreferences: saveInt + saveString]
    C --> E[notifyListeners]
    D --> E

    F[isLimited getter] --> G{_backendBusyUntilMs > now?}
    G -- yes --> H[return true]
    G -- no --> I{SharedPrefs until > now?}
    I -- yes --> H
    I -- no --> J[return false]

    K[reason getter] --> L{busyActive?}
    L -- yes --> M{rateActive AND persisted > inMemory?}
    M -- yes --> N[return rateLimit from prefs]
    M -- no --> O[return backendBusy]
    L -- no --> P{rateActive?}
    P -- yes --> N
    P -- no --> Q[return null]

    R[App Restart] --> S[_backendBusyUntilMs reset to 0]
    R --> T{SharedPrefs reason == backendBusy?}
    T -- yes --> U[Migration: clear stuck prefs]
    T -- no --> V[Keep rateLimit prefs intact]
Loading

Reviews (2): Last reviewed commit: "app(sync): align rate-limiter reason wit..." | Re-trigger Greptile

Comment on lines +57 to +74
DateTime? get until {
final ms = SharedPreferencesUtil().getInt(_prefKeyUntil);
return ms > 0 ? DateTime.fromMillisecondsSinceEpoch(ms) : null;
final now = DateTime.now().millisecondsSinceEpoch;
final persisted = SharedPreferencesUtil().getInt(_prefKeyUntil);
final inMemory = _backendBusyUntilMs;
final candidates = <int>[if (inMemory > now) inMemory, if (persisted > now) persisted];
if (candidates.isEmpty) return null;
return DateTime.fromMillisecondsSinceEpoch(candidates.reduce((a, b) => a > b ? a : b));
}

RateLimitReason? get reason {
if (!isLimited) return null;
final name = SharedPreferencesUtil().getString(_prefKeyReason);
return RateLimitReason.values.asNameMap()[name] ?? RateLimitReason.rateLimit;
final now = DateTime.now().millisecondsSinceEpoch;
if (_backendBusyUntilMs > now) return RateLimitReason.backendBusy;
final persisted = SharedPreferencesUtil().getInt(_prefKeyUntil);
if (persisted > now) {
final name = SharedPreferencesUtil().getString(_prefKeyReason);
return RateLimitReason.values.asNameMap()[name] ?? RateLimitReason.rateLimit;
}
return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 reason and until can report an inconsistent pair when both cooldowns are active

When a backendBusy in-memory cooldown and a persisted rateLimit cooldown are both active (e.g., a stale-guard fired during a 429 window), until returns the max expiry (likely the rateLimit end time), while reason returns backendBusy because _backendBusyUntilMs > now takes priority. The UI would then show "Omi servers are busy — wait until [rateLimit deadline]", which is wrong on both counts: the labelled reason expires sooner than displayed, and the displayed deadline belongs to a different reason. After _backendBusyUntilMs expires, the reason silently flips to rateLimit with no change in until, but until that flip the user sees an incorrect message. The reason getter should return the reason whose expiry matches the value returned by until (i.e., the reason with the later deadline), consistent with until's max-based logic.

Copy link
Copy Markdown
Collaborator

@kodjima33 kodjima33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks — keeping backendBusy cooldown in-memory makes sense

When backendBusy and rateLimit cooldowns are both active, reason now returns the reason of whichever has the later deadline, matching until's max-based pick. Prevents showing a reason and deadline that refer to different cooldowns.
@mdmohsin7
Copy link
Copy Markdown
Member Author

@greptile-apps re-review

@mdmohsin7 mdmohsin7 merged commit 61bfc46 into main Jun 1, 2026
3 checks passed
@mdmohsin7 mdmohsin7 deleted the caleb/sync-busy-no-persist branch June 1, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants