Ubisam-MobileGit/tasks/038-settings-screen-reactive-auth-retry-push-toggle.md
choiminyeong 9a8f855e5e TASK-038: Settings screen with opt-in reactive auth retry and favorite-push toggle
Two features gated behind a new Settings screen so neither silently
changes existing behavior:

- Reactive auth retry (opt-in, default off): a live 401 despite
  proactive refresh already judging the token fine now optionally
  retries once with a forcibly-refreshed token before falling back to
  re-login. ReactiveTokenProvider is a separate interface from the
  existing fun interface BearerTokenProvider (not an addition to it),
  so every existing test fake keeps compiling; a new shared
  callAuthenticated helper (used by all four Gitea API classes instead
  of four duplicated retry implementations) checks for it and falls
  back to the old single-attempt behavior otherwise.
- Favorite push notifications (on/off, default on): TASK-036's
  background checker can now be turned off entirely, closing that
  task's own 'no in-app way to disable this' limitation.

Every new constructor parameter defaults to preserving old behavior
exactly, so no existing call site or test needed to change.

Live-verified: Settings screen opens from a new button, both switches
toggle and persist across navigation, no crash.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-29 18:11:12 +09:00

160 lines
11 KiB
Markdown

# TASK-038: Settings screen with an opt-in auth-retry toggle and a favorite-push on/off toggle
Status: implemented; all required checks pass; live-confirmed on the human product owner's device
Owner: Claude
Reviewer: Codex
## Outcome
Two features the human product owner explicitly asked for together, both gated behind a new
Settings screen so neither one silently changes existing behavior for everyone:
- **A — reactive auth retry (opt-in, default off)**: when a live 401 happens despite proactive
refresh already having judged the token not yet due (e.g. a server-side revocation, not ordinary
expiry), the app now optionally retries once with a forcibly-refreshed token instead of going
straight to the re-login screen. Off by default; the human product owner explicitly asked for
this to be something "설정화면에서 설정 해두면 동작하고 안해두면 동작 안하게."
- **B — favorite push notifications (on/off, default on)**: TASK-036's background push checker can
now be turned off entirely from Settings, closing that task's own "no in-app way to disable this"
known limitation.
## Scope
- Included: new `core/settings/` package (`AppSettingsStore` + `SharedPreferencesAppSettingsStore`,
`AppSettingsSession`), new `feature/settings/` package (`SettingsViewModel`, `SettingsScreen`),
`Route.Settings` + `RouteStackSaver` entry, a "설정" button on the repository list's top bar,
`FavoritePushCheckWorker` gated on B, a new `ReactiveTokenProvider` interface + `RefreshingTokenProvider.forceRefresh()`
+ shared `callAuthenticated` retry helper wired into all four Gitea API classes for A.
- Excluded: No per-repository granularity for B (a single app-wide toggle, not one per favorited
repository — see Design notes). No UI indication of *why* a request succeeded on retry (it's
silent by design — the whole point is the human product owner never sees anything when it works).
No change to the *proactive* refresh TASK-029 already does unconditionally — A only adds a
reactive fallback on top of it, opt-in.
## Design notes
- **`ReactiveTokenProvider` is a separate interface from `BearerTokenProvider`, not an addition to
it**: `BearerTokenProvider` is a `fun interface` (exactly one abstract method) specifically so
every existing test fake written as a plain `BearerTokenProvider { "token" }` lambda keeps
compiling — adding a second method to that interface would have broken every one of those across
the whole test suite. `ReactiveTokenProvider : BearerTokenProvider` adds the one extra capability
(`forceRefresh()`) reactive retry needs; only `RefreshingTokenProvider` (the one production
implementation) also implements it, and the shared retry helper checks
`tokenProvider as? ReactiveTokenProvider` before relying on it, falling back to the old
single-attempt behavior (a plain test fake, or the setting disabled) otherwise.
- **One shared `callAuthenticated` helper, not four separate copies of the same retry logic**: all
four Gitea API classes (`GiteaRepositoryBrowserApi`, `GiteaHistoryApi`, `GiteaWorkItemsApi`,
`GiteaNotificationsApi`) had near-identical `authenticated`/`get` wrappers already; each now
delegates the "get a token, call, retry once reactively if warranted" orchestration to one
function (`data/repositories/GiteaRepositoryBrowserApi.kt`, alongside `BearerTokenProvider` and
`ReactiveTokenProvider`, which already lived there and are already imported by the other three
API classes — not a new architectural pattern, just one more shared thing in that same spot).
- **`forceRefresh()` always attempts a refresh, unlike `accessToken()`'s proactive check**: the
whole reason the reactive path exists is that a 401 already happened despite the token looking
fine by the clock — calling `accessToken()` again reactively would just return the same
still-time-valid-looking token and retry identically. `forceRefresh()` skips that time check
entirely, but reuses the exact same mutex and refresh-or-fall-back-to-stale logic `accessToken()`
already had (factored into a shared private `refreshOrFallBack`), so a reactive retry racing an
in-flight proactive refresh (or another reactive retry) still shares one attempt at the single-use
rotating refresh token rather than each independently trying to consume it.
- **Every new constructor parameter defaults to preserving old behavior exactly**: `reactiveAuthRetryEnabled: () -> Boolean = { false }`
on all four API classes means every existing test/call site that doesn't pass it keeps compiling
and behaving identically; production wiring (each feature's `ViewModel.Factory`) passes
`{ AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }` instead. A lambda, not
the whole `AppSettingsStore` interface, so a test can flip retry on/off with a plain closure
without needing a fake store.
- **B is one app-wide toggle, not per-repository**: a per-repository toggle would need its own list
UI (every favorited repository, each with its own switch) and its own storage shape — real,
separable scope beyond what was asked for in this combined request. `FavoritePushCheckWorker`
checks the single toggle first, before touching the network at all, so turning it off also stops
the periodic check from doing any work, not just from posting.
- **Settings has no loading state**: `SettingsViewModel` reads `AppSettingsStore`'s synchronous
`SharedPreferences`-backed values directly at construction — unlike every other ViewModel in this
app, there's no network round trip to wait on, so the screen renders immediately.
## Acceptance criteria
- [x] A new "설정" button opens a Settings screen from the repository list's top bar.
- [x] Reactive auth retry defaults off; toggling it on/off persists across navigation and app
restarts (`SharedPreferences`).
- [x] Favorite push notifications defaults on (matching TASK-036's prior unconditional behavior);
toggling it off makes `FavoritePushCheckWorker` a no-op before any network call.
- [x] With reactive auth retry enabled and the token provider supporting it, a 401 triggers exactly
one retry with a forcibly-refreshed token; the original failure propagates if that retry also
fails, if the setting is off, or if the token provider doesn't support it.
- [x] Every existing API-class test and call site keeps compiling and passing unmodified (all new
constructor parameters are defaulted).
- [x] Build, unit tests, and lint pass.
- [x] Live-confirmed on the human product owner's device: the Settings screen opens, both switches
visibly toggle and correctly persist across back-navigation and re-entry, no crash.
## Security and privacy
- `forceRefresh()` performs the exact same refresh-token exchange TASK-029's proactive path already
does (same gateway call, same Keystore-backed `TokenVault`, same mutex) — this task adds a second
*trigger* for that existing, already-reviewed flow, not a new credential-handling code path.
- No new data is stored beyond two booleans in a new `SharedPreferences` file
(`ubgitea_app_settings`) — no tokens, no personal data.
- Reactive retry is opt-in and defaults off specifically because it changes how *every* API call in
the app behaves on a 401 — a security-adjacent code path was judged worth requiring an explicit,
visible choice rather than silently changing default behavior for everyone.
## Required checks
```text
./gradlew compileDebugKotlin compileDebugUnitTestKotlin
./gradlew testDebugUnitTest
./gradlew assembleDebug lintDebug
```
## Evidence
- `core/settings/AppSettingsStore.kt` (+ `SharedPreferencesAppSettingsStore`), `AppSettingsSession.kt`:
the two-toggle store and its shared-singleton accessor, mirroring `AuthSession`'s own pattern.
- `data/repositories/GiteaRepositoryBrowserApi.kt`: new `ReactiveTokenProvider` interface, new
shared `internal suspend fun callAuthenticated(...)`.
- `core/auth/android/RefreshingTokenProvider.kt`: implements `ReactiveTokenProvider`; new
`forceRefresh()`; `accessToken()`'s refresh/fallback logic factored into a shared private
`refreshOrFallBack` both methods now call.
- `GiteaRepositoryBrowserApi.kt`/`GiteaHistoryApi.kt`/`GiteaWorkItemsApi.kt`/`GiteaNotificationsApi.kt`:
each gained a defaulted `reactiveAuthRetryEnabled: () -> Boolean = { false }` constructor
parameter; each class's `authenticated`/`get` now delegates to `callAuthenticated`.
- `RepositoryListViewModel.kt`/`HistoryViewModel.kt`/`WorkItemsViewModel.kt`/`NotificationsViewModel.kt`/`CodeBrowserViewModel.kt`
(all five `Factory`s that construct one of the four API classes): now pass
`reactiveAuthRetryEnabled = { AppSettingsSession.store(application).isReactiveAuthRetryEnabled() }`.
- `core/notifications/FavoritePushCheckWorker.kt`: checks
`AppSettingsSession.store(application).isFavoritePushNotificationsEnabled()` first, before the
session check, before any network call.
- `feature/settings/SettingsViewModel.kt`, `SettingsScreen.kt`: the new screen and its ViewModel.
- `ui/navigation/Route.kt`, `RouteStackSaver.kt`: new `Route.Settings` + its encode/decode case.
- `feature/repositories/RepositoryListScreen.kt`, `MainActivity.kt`: the "설정" button and its
wiring into the route stack.
- New tests: `RefreshingTokenProviderTest.kt` gained `forceRefresh()` coverage (refreshes regardless
of stated expiry, falls back to stale on no-refresh-token/failed-refresh, empty when nothing
stored); new `CallAuthenticatedTest.kt` (disabled setting doesn't retry, an unsupporting token
provider doesn't retry, a successful retry uses the refreshed token, a failed retry propagates the
original failure, a blank forced-refresh result doesn't retry, a non-auth failure is never
retried); `RouteEncodingTest.kt` gained a `Route.Settings` round-trip case.
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL.
- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (every pre-existing test
across all four API classes and `RefreshingTokenProvider` unmodified and still green).
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors.
- Live device: installed over wireless `adb`. Opened Settings from the new button; both switches
visibly toggled (screenshot-confirmed each state change); backed out and re-entered Settings —
both toggles still reflected exactly what was set, confirming `SharedPreferences` persistence;
restored both to their defaults afterward; `logcat` clean of `FATAL EXCEPTION`/`AndroidRuntime`
throughout.
## Known limitations
- Reactive auth retry's actual effect (recovering from a live 401 without forcing re-login) hasn't
been observed end-to-end live — that requires a genuine 401 to occur with the setting on, which
is by nature a rare, hard-to-trigger event; covered instead by direct unit tests of the retry
orchestration (`CallAuthenticatedTest`) and of `forceRefresh()` itself.
- B remains one app-wide toggle, not per-repository (see Design notes) — deliberate, not an
oversight.
## Review findings
None yet — pending Codex/human review.