From 50d65b40c2e38739eb8f51ace6cb497cc950a049 Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Sun, 26 Jul 2026 12:09:35 +0200 Subject: [PATCH] docs: add design spec for improved first-connection flow --- ...6-07-26-improve-first-connection-design.md | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-26-improve-first-connection-design.md diff --git a/docs/superpowers/specs/2026-07-26-improve-first-connection-design.md b/docs/superpowers/specs/2026-07-26-improve-first-connection-design.md new file mode 100644 index 0000000..6c0ed2d --- /dev/null +++ b/docs/superpowers/specs/2026-07-26-improve-first-connection-design.md @@ -0,0 +1,163 @@ +# Improve the first connection page + +Status: approved, not yet implemented. + +## Problem + +`docs/IDEAS.md` backlog item: "improve first connection page: in +CreateProfile.tsx, in addition to the display name, we should directly ask +garmin login/password and manage MFA from there, so the synchronization +experience in profile page will be easier later on". + +Today, `CreateProfile.tsx` only asks for a display name (`POST /api/setup`) +and drops the user straight into the main app -- Garmin credentials are +filled in later, whenever the user happens to visit the Profile page. This +means a brand-new account can sit around fully "set up" but never actually +synced, with no prompt nudging the user to connect Garmin. + +## Design + +### Scope decision: mandatory, persisted gate + +Connecting Garmin becomes a **hard requirement** to enter the app, enforced +on every login, not just the first one -- confirmed explicitly, including +the corollary that this can't be a purely in-session check: account +provisioning (`POST /api/setup`) must happen before Garmin login can even be +attempted (the per-user Garmin client is keyed by an already-existing +account), so a user who abandons the flow mid-connect already has a +`users`/`profile` row. Enforcing the gate on *every subsequent login* (not +just immediately after signup) requires a persisted "has this account ever +successfully connected" flag -- the in-memory `Server.userAuthStatus` map +resets on every backend restart and can't be trusted for this. + +Also confirmed: both `CreateProfile` and the new gate screen get a "Log out" +link (neither has one today), since a mandatory gate a user can get +genuinely stuck on (no Garmin credentials handy right now) needs an escape +hatch back to the login screen. + +### Data model + +`schema.sql`, `profile` table -- one new nullable column: + +```sql + -- Set once, the first time this user successfully authenticates with + -- Garmin (handleAuthLogin/handleAuthMFA). Null means never connected -- + -- the login gate uses this (not the in-memory auth status, which + -- resets on restart) to decide whether a returning user must + -- reconnect before entering the app. + garmin_connected_at TEXT, +``` + +`store.Profile` gets a matching `GarminConnectedAt *string` field (same +nullable-TEXT-pointer pattern as `sync_state.EarliestSyncedDate`). + +New store method (`internal/store/users.go` or a new small file, e.g. +`profile.go` wherever `UpdateProfile`/`GetProfile` already live): + +```go +// MarkGarminConnected records the first time userID successfully +// authenticates with Garmin. A no-op if already set, so it always reflects +// the first connection, not the most recent one. +func (db *DB) MarkGarminConnected(ctx context.Context, userID int64) error +``` + +Implemented as `UPDATE profile SET garmin_connected_at = datetime('now') +WHERE user_id = ? AND garmin_connected_at IS NULL`. + +After the schema edit, regenerate `docs/DATABASE.md` via `go run +./cmd/dumpschema` (per project convention). + +### Backend API + +- **`handleAuthLogin`/`handleAuthMFA`** (`internal/api/auth.go`): right after + the existing `s.recordAuthResult(userID, res)` call, if `res.Status == + garmin.AuthSuccess`, call `s.DB.MarkGarminConnected(r.Context(), userID)`. + A failure here is logged and does *not* fail the HTTP response -- the + Garmin authentication itself already succeeded; failing to persist the + flag is a non-fatal side effect (the next successful login attempt would + just set it then). +- **`sessionMeResponse`** (`internal/api/session.go`): add `GarminConnected + bool \`json:"garmin_connected"\``. +- **`handleSessionMe`**: when the session already has a profile, fetch it + (`s.DB.GetProfile`) and set `resp.GarminConnected = profile.GarminConnectedAt + != nil`. +- No other endpoint changes. The new frontend screen calls the exact same + `PUT /api/profile`, `POST /api/auth/login`, `POST /api/auth/mfa` the + existing Profile page already uses. + +### Frontend flow + +- **`types/api.ts`**: `SessionInfo` gains `garmin_connected: boolean`. +- **`LoginGate.tsx`**: becomes a three-way fork instead of two: + ``` + !authenticated -> login screen + authenticated && !has_profile -> CreateProfile + authenticated && has_profile && !garmin_connected -> ConnectGarmin + authenticated && has_profile && garmin_connected -> App + ``` + `CreateProfile.onCreated` and the new `ConnectGarmin.onConnected` both just + flip the relevant boolean in the local `session` state object (same + pattern `has_profile` already uses). +- **`CreateProfile.tsx`**: behavior unchanged (display name -> + `POST /api/setup`); it's simply no longer the last screen. Gains a + "Log out" link (`
`, + same pattern as `App.tsx`'s header). +- **New `frontend/src/ConnectGarmin.tsx`**: a dedicated, self-contained + onboarding screen (not a reuse of `GarminConnection.tsx`, to avoid + coupling that shared Profile-page component to an onboarding-only special + case): + - Email + password fields. + - "Connect" button: `PUT /api/profile` (persist credentials into the + profile row created by setup) then `POST /api/auth/login`. + - If the result is `mfa_required`, show an MFA code field -> `POST + /api/auth/mfa`. + - On `authenticated`, show a brief confirmation and a "Continue to + geniusrun" button that calls `onConnected()` -- no sync is + auto-triggered here; the existing background incremental-sync loop and + the Profile page's "Sync now" handle actual data fetching, keeping this + screen scoped to connecting only. + - On `failed` (wrong password, Garmin unreachable, rate-limited), show the + returned message and leave the fields editable for a retry -- never a + dead end. + - Also gains a "Log out" link. + +### Error handling + +- Wrong credentials / Garmin down / rate-limited: surfaced via the same + `authResponse.message` field `GarminConnection.tsx` already displays; + fields stay editable, retry is always available. +- `MarkGarminConnected` DB failure: logged server-side, never surfaced to + the user and never fails the auth response (see Backend API above). +- Abandoning the flow (closing the tab) leaves the account provisioned but + not connected -- the *next* login re-enters at `ConnectGarmin` (that's the + whole point of persisting the flag), not at `CreateProfile` again. + +### Testing + +- `internal/store`: `MarkGarminConnected` sets the timestamp exactly once + (a second call is a no-op, preserving the first-connection time); + adversarial isolation test that marking user A connected never affects + user B's `GarminConnectedAt`. +- `internal/api`: `POST /api/auth/login` (via `mock.Client` returning + `AuthSuccess`) followed by `GET /api/session/me` reports `garmin_connected: + true`; an `mfa_required` or `failed` result does *not* set it; a freshly + provisioned user (via `newTestServer`, no Garmin attempt yet) reports + `garmin_connected: false`. +- No frontend test suite exists yet (per `CLAUDE.md`) -- manual smoke test: + fresh account -> display name -> forced into `ConnectGarmin` -> wrong + password shows an error and allows retry -> correct password (or MFA) + resolved -> lands in `` -> log out -> log back in with the same + account -> confirm it goes straight to `` (not back through + `ConnectGarmin`), proving `garmin_connected_at` actually persisted. + +## Out of scope + +- Auto-triggering a first sync from `ConnectGarmin` -- deferred to the + existing "Sync now" / background incremental-sync mechanisms. +- Any "reconnect required" re-gating if Garmin credentials later go stale + (e.g. password changed on Garmin's side) -- this feature only covers the + *first-ever* successful connection; subsequent reconnects still go through + the normal Profile page flow. +- Combining the display-name and Garmin-credential steps into a single + form/request -- kept as two sequential screens since Garmin login + requires an already-provisioned account (see Scope decision above).