Ubisam-MobileGit/tasks/013-typed-navigation.md

TASK-013: Typed application navigation

Status: implemented; all required checks pass; no review pending (Codex retired, 2026-09-29) Owner: Claude (human product owner directed Claude to own all subsequent tasks, 2026-08-26) Reviewer: Codex

Outcome

Replace MainActivity's ad-hoc browsingCode/browsingHistory/browsingWorkItems booleans with a typed route stack that survives configuration change and process recreation, with well-defined back behavior between the repository list, code browser, history, and work-items screens.

Scope

• Included:
  • A Route sealed type in ui/navigation/ holding only stable primitives (owner, repo name, ref kind/value, path, item kind/number) — never tokens, authorization codes, or fetched content.
  • A back stack (List<Route>) owned by MainActivity, restorable through configuration change and process death via a rememberSaveable custom Saver.
  • On any route becoming current (fresh navigation or restoration), the owning feature ViewModel is told to (re)load that route's target — this re-fetches content rather than persisting fetched data itself, consistent with the rest of the app's no-offline-cache posture.
  • Back navigation pops the route stack; a feature's own internal back behavior (e.g. code browser directory-up, history detail-to-list) is unchanged and still owned by that feature's ViewModel.
  • Authentication expiry clears the route stack back to Repositories in addition to the existing per-feature close() + logout behavior.
• Excluded:
  • Restoring deep in-feature state (which file is open mid-scroll, which page of a paginated list, which review is expanded) — only the route's identifying target is restored; the feature reloads fresh from there. This matches the explicit scope note in the prior serial-handoff roadmap ("Current screen restoration is not a full typed back stack; TASK-013 owns it" referred to route-level restoration, not full UI-state persistence).
  • Migrating to Navigation Compose or any other new dependency — the app's screen graph is four shallow, mostly-linear screens; a hand-rolled sealed Route + List<Route> stack is sufficient and avoids a dependency AGENTS.md would otherwise require justifying.

Writable paths

• app/src/main/java/com/ubisam/ubgitea/ui/navigation/
• app/src/test/java/com/ubisam/ubgitea/ui/navigation/
• app/src/main/java/com/ubisam/ubgitea/MainActivity.kt
• tasks/013-typed-navigation.md

Acceptance criteria

• Route stores only stable primitives; no token, authorization code, or fetched-content field exists on any route — Route is Repositories | CodeBrowser(owner,name,refKind,ref) | History(owner,name,ref) | WorkItems(owner,name), all String fields.
• The route stack is restorable through configuration change and process recreation (custom Saver) — RouteStackSaver encodes each route as a delimited string (List<String> is directly Bundle-safe), using U+001F as the field separator, which RepositoryCoordinates/RepositoryRef already guarantee cannot appear in real field values (both reject ISO control characters).
• Back pops exactly one route; popping to a CodeBrowser/History/WorkItems route re-triggers that feature's load for the restored target instead of showing stale/empty state — a single LaunchedEffect(currentRoute) in MainActivity calls the owning ViewModel's open(...) whenever the top-of-stack route changes, for both fresh navigation and restoration (they're indistinguishable to this effect, which is the point).
• Authentication expiry clears the route stack back to Repositories — one shared authenticationExpired() closes every feature ViewModel, resets the stack to [Repositories], and logs out; wired identically from all three feature screens' onAuthenticationExpired.
• Tests cover route encoding/decoding round-trip (the Saver) and back-stack push/pop behavior — 6 tests including a ref containing /, an unrecognized-encoding fallback, and a full multi-route stack round-tripped through the actual Saver.save/restore functions (not just the internal encode/decode helpers).

Required checks

./gradlew assembleDebug
./gradlew testDebugUnitTest
./gradlew lintDebug

Evidence

Implemented on branch claude/task-013-navigation from commit c397fbe (includes TASK-007/008/009/010, none yet Codex-reviewed).

• New: ui/navigation/Route.kt, ui/navigation/RouteStackSaver.kt, RouteEncodingTest.kt. Rewrote: MainActivity.kt — replaced the three independent rememberSaveable booleans (browsingCode/browsingHistory/browsingWorkItems) with one List<Route> back stack.
• Genuine behavior improvement, not just a refactor: the old booleans could only model two fixed nesting paths (code-browser→history was correct only because browsingCode happened to stay true underneath browsingHistory; work-items had no nesting). The new stack is a real LIFO — pushing History on top of CodeBrowser on top of Repositories means back from history lands on code browser, back from code browser lands on repositories, and any future screen added to the stack gets correct back behavior for free instead of needing another hand-wired boolean and another special case in the nested if/else.
• listSaver's actual Compose signature returns Saver<List<T>, Any> (not Saver<List<T>, List<Saveable>> as its type parameters might suggest) — this was discovered by letting the compiler's type-mismatch error report the real inferred type rather than guessing, and RouteStackSaver's declared type was corrected to match.
• Feature-internal back behavior (code browser directory-up, history detail-to-list, work-items detail-to-list) is unchanged — it is still owned by each feature's own ViewModel and is not part of the route stack, consistent with this task's explicitly bounded scope.

Test coverage (6 cases in RouteEncodingTest): Repositories round-trip (1), CodeBrowser round-trip including a ref containing / (1), History round-trip (1), WorkItems round-trip (1), an unrecognized encoding falls back to Repositories instead of throwing (1), a full three-route stack round-tripped through the real Saver.save/restore functions in order (1).

Checks run from the new worktree (local.properties created locally, not committed):

./gradlew testDebugUnitTest --tests "com.ubisam.ubgitea.ui.navigation.RouteEncodingTest"   → BUILD SUCCESSFUL, 6/6 passed
./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks                            → BUILD SUCCESSFUL
  - 164 unit tests across 28 suites, 0 failures/errors
  - 0 lint errors
git diff --check                                                                              → clean (CRLF notice only)
sensitive-pattern scan of changed files                                                       → no matches

Known limitations:

• Only the route (which screen + its identifying target) survives process death, not deeper in-feature state (open commit, scroll position, which page) — explicitly excluded in Scope above.
• No physical device was available to verify process-death restoration interactively (this can only be fully confirmed with "don't keep activities" + an actual kill, which requires a device); the Saver round-trip itself is unit-tested directly instead.

No open P0-P3 findings from self-review. Requesting Codex read-only review.