Skip to content

Harden init resilience: nuclear reset, local-only startup, auth caching#112

Open
dougborg wants to merge 4 commits intoFoggedLens:mainfrom
dougborg:fix/harden-init-resilience
Open

Harden init resilience: nuclear reset, local-only startup, auth caching#112
dougborg wants to merge 4 commits intoFoggedLens:mainfrom
dougborg:fix/harden-init-resilience

Conversation

@dougborg
Copy link
Collaborator

@dougborg dougborg commented Feb 12, 2026

Summary

  • Sequential init with nuclear reset: if init fails >= 2 consecutive times, wipe all data and start fresh; prevents stuck loading screen on Android. Exposes consumeDidNuclearReset() so the UI can inform the user.
  • Split suspected location init into local-only initLocal() + background refreshIfNeeded() — no network during startup
  • Harden auth: 10s timeout on _fetchUsername(), per-mode display name caching (cached_display_name_production / _sandbox), restoreLoginLocal() for local-only init, _TokenRejectedException for 401/403 handling that clears stale tokens
  • Lazy OAuth2Helper: helper is only constructed on first login()/forceLogin() call, so keys.dart is never touched during tests or local-only startup
  • Auth and suspected location state get DI constructors for testability
  • NuclearResetService cleaned up (removed unused singleton pattern)
  • Delete accidental COMMENT file

Addresses feedback from #106 (items 1-3, 5). Item 4 (parallelization) is in PR #107.

Test plan

  • flutter analyze — no issues
  • flutter test — all 117 tests pass (33 new)
  • Manual test on Android device: verify app loads past splash screen
  • Verify nuclear reset triggers after 2 consecutive init failures (can simulate by corrupting SharedPreferences)
  • Verify cached display name shows immediately on startup, then refreshes in background

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings February 12, 2026 19:37
@dougborg dougborg force-pushed the fix/harden-init-resilience branch from 24064f3 to ae0df2e Compare February 12, 2026 19:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens app startup to avoid the Android “stuck on splash/loading screen” scenario by eliminating network calls during initialization, adding background refresh paths for auth and suspected-locations data, and introducing a “nuclear reset” recovery after repeated init failures.

Changes:

  • Split startup flows into local-only init (initLocal() / restoreLoginLocal()) plus background refresh (refreshIfNeeded()), including a 10s timeout and display-name caching for auth.
  • Add init failure tracking with a nuclear reset (wipe all local data) after 2 consecutive init failures, and wrap _init() in a top-level try/catch to avoid permanent loading screens.
  • Add new unit tests for AuthService, AuthState, and SuspectedLocationState.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/app_state.dart Sequential init now has failure tracking + nuclear reset and moves network work to post-init background tasks.
lib/services/auth_service.dart Adds HTTP timeout, per-mode cached display name, local-only restore, and token-rejection handling.
lib/state/auth_state.dart Uses local-only restore during init and adds background refresh method.
lib/services/suspected_location_service.dart Adds initLocal() and background refreshIfNeeded() to avoid network during startup.
lib/state/suspected_location_state.dart Adds local-only init and background refresh wrapper with loading state management.
lib/keys.dart Returns empty strings when dart-defines are missing (test support) and logs warnings.
test/services/auth_service_test.dart Adds coverage for auth restore, caching, per-mode isolation, and logout behavior.
test/state/auth_state_test.dart Adds coverage for init/refresh/login/logout and upload-mode changes.
test/state/suspected_location_state_test.dart Adds coverage for initLocal, refresh loading transitions, and selection behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dougborg pushed a commit to dougborg/deflock-app that referenced this pull request Feb 12, 2026
- Fix stale nuclear reset counter: re-read from prefs after wipe
- Remove fake loading state from refreshIfNeeded (background refresh)
- Clear stale username when token expires between init and refresh
- Fail fast in release builds when OAuth client IDs missing (keys.dart)
- Update tests to match new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dougborg pushed a commit to dougborg/deflock-app that referenced this pull request Feb 25, 2026
- Fix stale nuclear reset counter: re-read from prefs after wipe
- Remove fake loading state from refreshIfNeeded (background refresh)
- Clear stale username when token expires between init and refresh
- Fail fast in release builds when OAuth client IDs missing (keys.dart)
- Update tests to match new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dougborg pushed a commit to dougborg/deflock-app that referenced this pull request Feb 25, 2026
- Fix stale nuclear reset counter: re-read from prefs after wipe
- Remove fake loading state from refreshIfNeeded (background refresh)
- Clear stale username when token expires between init and refresh
- Fail fast in release builds when OAuth client IDs missing (keys.dart)
- Update tests to match new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dougborg dougborg force-pushed the fix/harden-init-resilience branch from c34c58a to e91c5d6 Compare February 25, 2026 17:15
dougborg pushed a commit to dougborg/deflock-app that referenced this pull request Feb 25, 2026
- Fix stale nuclear reset counter: re-read from prefs after wipe
- Remove fake loading state from refreshIfNeeded (background refresh)
- Clear stale username when token expires between init and refresh
- Fail fast in release builds when OAuth client IDs missing (keys.dart)
- Update tests to match new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dougborg pushed a commit to dougborg/deflock-app that referenced this pull request Feb 25, 2026
- Fix stale nuclear reset counter: re-read from prefs after wipe
- Remove fake loading state from refreshIfNeeded (background refresh)
- Clear stale username when token expires between init and refresh
- Fail fast in release builds when OAuth client IDs missing (keys.dart)
- Update tests to match new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dougborg dougborg force-pushed the fix/harden-init-resilience branch 2 times, most recently from baecebb to beb8b03 Compare February 25, 2026 22:54
Add initLocal() for fast local-only init (SharedPrefs + SQLite) and
refreshIfNeeded() for background network fetch when data is stale.
Add DI constructor to SuspectedLocationState for testability.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dougborg dougborg force-pushed the fix/harden-init-resilience branch 3 times, most recently from ef9293b to 4b23b27 Compare February 26, 2026 00:12
Copy link
Collaborator

@stopflock stopflock left a comment

Choose a reason for hiding this comment

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

Most "if it works im happy" - commented a couple places.

dougborg and others added 3 commits February 25, 2026 17:54
- Add 10-second timeout on _fetchUsername() HTTP call
- Add injectable http.Client for testability (default: UserAgentClient)
- Cache display name to SharedPreferences per upload mode
- Add _TokenRejectedException for 401/403 handling
- restoreLogin(): on 401/403 logout, on network error fall back to cache
- Add restoreLoginLocal(): restore from cache only (no network) for init
- AuthState.init() now uses restoreLoginLocal() for fast startup
- Add AuthState.refreshIfNeeded() for background token validation
- logout() clears cached display name
- Remove dead validateToken() method; simplify onUploadModeChanged

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Nuclear reset: if init has failed >= 2 times, wipe all app data via
  NuclearResetService.clearEverything() before retrying
- Nuclear reset now also clears flutter_secure_storage (OAuth tokens)
- Increment failure count before init, clear on success
- Keep init sequential (no Future.wait) to avoid race conditions
- Use local-only init for suspected locations and auth (no network)
- Fire-and-forget background refresh after init completes
- On catch: set _isInitialized = true to prevent stuck loading screen
- Fix stale nuclear reset counter: re-read from prefs after wipe

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
AuthService: restoreLogin caching/fallback, restoreLoginLocal,
isLoggedIn, getAccessToken, logout, mode isolation
AuthState: init uses local restore, refreshIfNeeded, login, logout,
mode changes (including null/rejected token scenarios)
SuspectedLocationState: initLocal (including error resilience),
refreshIfNeeded, selection

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +188 to +191
// Only clear oauth2_client tokens if the helper was already initialized
// (i.e., an OAuth flow ran in this mode). If _helper is null, no tokens
// exist in the library's secure storage for the current mode.
await _helper?.removeAllTokens();
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

logout() only calls removeAllTokens() when _helper is already initialized. Because _helper is now lazy, it's possible to log out without ever constructing the helper in the current run, leaving oauth2_client tokens in flutter_secure_storage. That can undermine logout semantics (next login()/getToken() may reuse the stored token). Consider always clearing oauth2_client tokens on logout (e.g., call _oauthHelper.removeAllTokens() and handle missing keys gracefully, or directly clear flutter_secure_storage entries used by oauth2_client).

Suggested change
// Only clear oauth2_client tokens if the helper was already initialized
// (i.e., an OAuth flow ran in this mode). If _helper is null, no tokens
// exist in the library's secure storage for the current mode.
await _helper?.removeAllTokens();
// Always clear oauth2_client tokens via the helper, even if it needs
// to be lazily initialized, so that logout semantics are consistent
// across app runs and modes.
await _oauthHelper.removeAllTokens();

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

yes? no?

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.

3 participants