ADR: fix-mobile-auth-callback-route

ADR: fix-mobile-auth-callback-route

Context

OAuth deep-links into gym_app hit a GoException: no routes for location: com.fitspace.client.app://auth/callback?code=<uuid> because the router has no matching route and no errorBuilder. The guard short-circuit on /auth/callback never fires (GoRouter’s redirect only runs for matched routes). Root cause and confirmation are in the spec — see specs/fix-mobile-auth-callback-route.md.

Decision

1. Add a GoRoute(path: '/auth/callback', builder: …) to apps/gym_app/lib/router/app_router.dart whose builder renders only a centered CircularProgressIndicator on a Scaffold and emits one Logger().info('PKCE callback received') line. The builder does NOT touch the Supabase SDK and does NOT call context.go(...).
2. Add an errorBuilder: on the same GoRouter instance. In release builds it redirects (via WidgetsBinding.instance.addPostFrameCallback → context.go(...)) to /auth/welcome if auth.isAuthenticated == false, else /home/main. In non-release builds (!kReleaseMode) it falls back to the default GoException widget so genuine routing bugs stay visible during development.
3. Remove the /auth/callback short-circuit at packages/auth/lib/src/auth_guard.dart:19 — the new matched route makes it dead code. The remaining guard logic (empty path / root → home or welcome by auth state) is kept untouched.

Considered alternatives

A. Call supabase.auth.exchangeCodeForSession(code) from the route builder

Rejected. supabase_flutter 2.12.4 is initialised with detectSessionInUrl: true (default), which wires an app_links listener that consumes the URI and performs the PKCE exchange itself. A second manual call races the SDK and throws "code already exchanged" (or worse, succeeds first and leaves the SDK’s internal listener in a half-initialised state). The route builder must be a passive landing pad.

B. Navigate explicitly via context.go('/home/main') from the callback builder

Rejected. Navigation post-exchange is already driven by onAuthStateChange → AuthChangeEvent.signedIn at auth_service.dart:69, which calls notifyListeners() and triggers the refreshListenable: auth on the GoRouter (app_router.dart:65). The guard then redirects based on isAuthenticated. An explicit context.go fires before the session is fully written to secure storage and competes with the listener — flaky in practice, intermittently lands users back on /auth/welcome.

C. Keep the guard short-circuit and just add the route

Rejected. Once a real route exists, the short-circuit becomes unreachable code that future maintainers will misread as load-bearing. The redirect chain runs through the normal if (!isAuth && !isOnAuthPage) branch — /auth/callback already starts with /auth, so unauthenticated users won’t be bounced mid-exchange.

D. Rewrite the guard short-circuit to handle the route explicitly

Rejected for the same reason as C plus added complexity. The new code path is strictly simpler — fewer branches, fewer special cases.

Surface impact

N/A — spec frontmatter has surfaces: []; no contract change.

API design (per surface)

N/A — spec frontmatter has surfaces: []; no contract or schema change.

Data model

N/A — spec frontmatter has surfaces: []; no contract or schema change.

Backend module placement

N/A — spec frontmatter has surfaces: []; no backend change.

Frontend implications (per app)

• tktspace-business: no change.
• tktspace-web: no change.
• tktspace-landing: no change.

Mobile implications

• App affected: apps/gym_app only. tickets_app does not use Supabase OAuth and has no /auth/callback deep link.
• Shared packages affected: packages/auth (one-line removal in auth_guard.dart).
• Touch list:
  • apps/gym_app/lib/router/app_router.dart — add GoRoute('/auth/callback')
    • errorBuilder: on the GoRouter.
  • packages/auth/lib/src/auth_guard.dart:19 — drop the loc.startsWith('/auth/callback') term.
  • apps/gym_app/test/router/auth_callback_route_test.dart (new) — Phase B widget test (AC-6).
• No native / Podfile / Info.plist changes. The com.fitspace.client.app:// URI scheme is already registered for the existing app_links pipeline.
• No new package deps. go_router, provider, supabase_flutter, tktspace_core (Logger) are already present in gym_app.
• Deep-linking: unchanged at the OS layer. This ADR only fixes the in-router landing once app_links hands the URI off to GoRouter.
• Offline considerations: N/A — OAuth callback requires network by definition.

Interaction with sibling ticket 869dkrxau

Sibling ticket 869dkrxau (fix-mobile-supabase-session-restore-race) plans to introduce an explicit hydration signal so the router waits for Supabase to restore the session before evaluating the guard. The interaction:

• This ticket does not depend on 869dkrxau. The callback flow works correctly today regardless of whether AuthService.notifyListeners() fires during cold start, because supabase_flutter consumes the PKCE code via its own app_links listener and emits signedIn once the exchange finishes — the listener then notifies our AuthService which refreshes the router.
• Once 869dkrxau lands, the /auth/callback loading scaffold will benefit for free: the hydration signal makes the guard’s “should I redirect away?” decision more deterministic, removing the (rare) flash of /auth/welcome if signedIn fires before the in-flight redirect resolves.
• Ordering: these tickets can land in either order. If 869dkrxau lands first, no change to this ADR. If this lands first, 869dkrxau’s hydration flag layers on top without revisiting the route or the guard.

Risks & mitigations

• errorBuilder swallows legitimate routing bugs in dev. Mitigation: gate the redirect-on-unmatched behaviour behind kReleaseMode. Dev and profile builds fall through to GoRouter’s default GoException widget so typos and missing routes still surface loudly. Release builds redirect quietly to /auth/welcome (unauthed) or /home/main (authed).
• PKCE exchange takes longer than expected, loader visible >1s on slow networks. Mitigation: none — accepted by the spec. AC-5’s Logger().info('PKCE callback received') line gives us a production timing signal to measure exchange latency and decide if a future iteration needs better UX.
• Guard removal regression. Mitigation: the new matched route covers the exact location the removed short-circuit targeted; the rest of the guard still handles loc.isEmpty || loc == '/'. Widget test in AC-6 + a one-time manual OAuth round-trip cover the regression surface.
• errorBuilder redirect runs during build. Mitigation: schedule the context.go(...) inside WidgetsBinding.instance.addPostFrameCallback to avoid “navigated during build” assertions. Render a transient empty Scaffold while the post-frame callback fires.

Rollout plan

• Feature flag: none. Bug fix to a broken happy-path; gating would just keep users on the broken path.
• Phased: no. Single-PR fix in tktspace-mobile-app targeted at gym_app, shipped to TestFlight / internal Play track first per the repo’s standard mobile release cadence.
• Backfill: N/A.
• Removal of AC-5 log line: the Logger().info('PKCE callback received') call is intentionally temporary. Open a follow-up ticket to drop it once one release confirms the fix in prod (per AC-5 wording).

Test approach

• Unit / widget (Phase B): single testWidgets() in apps/gym_app/test/router/auth_callback_route_test.dart that:
  1. Builds the GoRouter from buildRouter(FakeAuthService()) wrapped in a MaterialApp.router.
  2. Calls router.go('/auth/callback?code=fake').
  3. Pumps and asserts: no GoException widget, exactly one CircularProgressIndicator, and no thrown exceptions. No simulator and no real Supabase client — FakeAuthService just extends ChangeNotifier and returns false for isAuthenticated.
• Manual: post-merge, one OAuth round-trip on a real device per provider (Google + Apple) to confirm AC-1 / AC-2 against the dev backend.
• No backend, contract, or DB tests — none of those surfaces change.

STATUS: READY_FOR_REVIEW