Merge branch 'worktree-improve-first-connection'

backend/internal/api/api_test.go

@@ -765,6 +765,56 @@ func TestProfile_RejectsInvalidHRZones(t *testing.T) {
 	}
 }
 
+func TestSessionMe_ReportsGarminConnectedAfterSuccessfulAuth(t *testing.T) {
+	s, _, _ := newTestServer(t)
+	router := s.Router()
+
+	rec := doJSON(t, router, http.MethodGet, "/api/session/me", nil)
+	var before sessionMeResponse
+	unmarshalBody(t, rec, &before)
+	if before.GarminConnected {
+		t.Fatal("expected a fresh account to report garmin_connected=false")
+	}
+
+	rec = doJSON(t, router, http.MethodPost, "/api/auth/login", nil) // mock.Client defaults to AuthSuccess
+	if rec.Code != http.StatusOK {
+		t.Fatalf("auth/login status = %d, body = %s", rec.Code, rec.Body.String())
+	}
+
+	rec = doJSON(t, router, http.MethodGet, "/api/session/me", nil)
+	var after sessionMeResponse
+	unmarshalBody(t, rec, &after)
+	if !after.GarminConnected {
+		t.Fatal("expected garmin_connected=true after a successful auth")
+	}
+}
+
+func TestSessionMe_GarminConnectedStaysFalseOnMFARequiredOrFailed(t *testing.T) {
+	s, _, userID := newTestServer(t)
+	client, err := s.garminFor(context.Background(), userID)
+	if err != nil {
+		t.Fatalf("garminFor: %v", err)
+	}
+	mockClient, ok := client.(*mock.Client)
+	if !ok {
+		t.Fatalf("expected *mock.Client, got %T", client)
+	}
+	mockClient.AuthResults = []garmin.AuthResult{{Status: garmin.AuthMFARequired, Message: "MFA required"}}
+
+	router := s.Router()
+	rec := doJSON(t, router, http.MethodPost, "/api/auth/login", nil)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("auth/login status = %d, body = %s", rec.Code, rec.Body.String())
+	}
+
+	rec = doJSON(t, router, http.MethodGet, "/api/session/me", nil)
+	var me sessionMeResponse
+	unmarshalBody(t, rec, &me)
+	if me.GarminConnected {
+		t.Fatal("expected garmin_connected=false after mfa_required")
+	}
+}
+
 func TestDeleteProfile_DeletesUserAndTerminatesGarminClient(t *testing.T) {
 	s, db, userID := newTestServer(t)
 	router := s.Router()

backend/internal/api/auth.go

@@ -1,7 +1,9 @@
 package api
 
 import (
+	"context"
 	"encoding/json"
+	"log"
 	"net/http"
 
 	"geniusrun/backend/internal/garmin"
@@ -25,11 +27,22 @@ func authStatusString(s garmin.AuthStatus) string {
 	}
 }
 
-func (s *Server) recordAuthResult(userID int64, res garmin.AuthResult) {
+// recordAuthResult updates the in-memory auth status/message for userID,
+// and -- on a successful authentication -- persists that this account has
+// connected to Garmin at least once (store.MarkGarminConnected), which is
+// what the login gate actually checks (the in-memory auth status resets on
+// every backend restart; this doesn't).
+func (s *Server) recordAuthResult(ctx context.Context, userID int64, res garmin.AuthResult) {
 	s.mu.Lock()
 	s.userAuthStatus[userID] = res.Status
 	s.userAuthMessage[userID] = res.Message
 	s.mu.Unlock()
+
+	if res.Status == garmin.AuthSuccess {
+		if err := s.DB.MarkGarminConnected(ctx, userID); err != nil {
+			log.Printf("api: mark garmin connected for user %d: %v", userID, err)
+		}
+	}
 }
 
 func (s *Server) handleAuthLogin(w http.ResponseWriter, r *http.Request) {
@@ -44,7 +57,7 @@ func (s *Server) handleAuthLogin(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadGateway, err.Error())
 		return
 	}
-	s.recordAuthResult(userID, res)
+	s.recordAuthResult(r.Context(), userID, res)
 	writeJSON(w, http.StatusOK, authResponse{Status: authStatusString(res.Status), Message: res.Message})
 }
 
@@ -72,7 +85,7 @@ func (s *Server) handleAuthMFA(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadGateway, err.Error())
 		return
 	}
-	s.recordAuthResult(userID, res)
+	s.recordAuthResult(r.Context(), userID, res)
 	writeJSON(w, http.StatusOK, authResponse{Status: authStatusString(res.Status), Message: res.Message})
 }
 

backend/internal/api/isolation_test.go

@@ -181,3 +181,25 @@ func TestIsolation_DeleteProfileOnlyDeletesOwnAccount(t *testing.T) {
 		t.Fatalf("userA profile status after userB deleted themselves = %d, body = %s", rec.Code, rec.Body.String())
 	}
 }
+
+// TestIsolation_GarminConnectedNeverLeaksAcrossUsers confirms one user's
+// successful Garmin auth never flips garmin_connected for another user.
+func TestIsolation_GarminConnectedNeverLeaksAcrossUsers(t *testing.T) {
+	s, db, _ := newTestServer(t) // provisions "test-user" (userA)
+	if _, err := db.ProvisionUser(newCtx(), "user-b", "B"); err != nil {
+		t.Fatalf("ProvisionUser(b): %v", err)
+	}
+
+	router := s.Router()
+	rec := doJSON(t, router, http.MethodPost, "/api/auth/login", nil) // as userA
+	if rec.Code != http.StatusOK {
+		t.Fatalf("auth/login status = %d, body = %s", rec.Code, rec.Body.String())
+	}
+
+	rec = doJSONAs(t, router, "user-b", http.MethodGet, "/api/session/me", nil)
+	var meB sessionMeResponse
+	unmarshalBody(t, rec, &meB)
+	if meB.GarminConnected {
+		t.Fatal("userA's successful Garmin auth leaked into userB's garmin_connected flag")
+	}
+}

backend/internal/api/session.go

@@ -32,10 +32,11 @@ type SessionConfig struct {
 }
 
 type sessionMeResponse struct {
-	Name        string `json:"name"`
+	Name            string `json:"name"`
-	Email       string `json:"email"`
+	Email           string `json:"email"`
-	HasProfile  bool   `json:"has_profile"`
+	HasProfile      bool   `json:"has_profile"`
-	DisplayName string `json:"display_name,omitempty"`
+	DisplayName     string `json:"display_name,omitempty"`
+	GarminConnected bool   `json:"garmin_connected"`
 }
 
 func (s *Server) handleSessionLogin(w http.ResponseWriter, r *http.Request) {
@@ -113,6 +114,12 @@ func (s *Server) handleSessionMe(w http.ResponseWriter, r *http.Request) {
 	if u, found := userFromContext(r.Context()); found {
 		resp.HasProfile = true
 		resp.DisplayName = u.DisplayName
+		profile, err := s.DB.GetProfile(r.Context(), u.ID)
+		if err != nil {
+			writeError(w, http.StatusInternalServerError, err.Error())
+			return
+		}
+		resp.GarminConnected = profile.GarminConnectedAt != nil
 	}
 	writeJSON(w, http.StatusOK, resp)
 }

backend/internal/store/isolation_test.go

@@ -204,3 +204,31 @@ func TestIsolation_DeleteUserLeavesOtherUsersDataIntact(t *testing.T) {
 		t.Fatalf("userB's activities affected by deleting userA: len=%d err=%v", len(activitiesB), err)
 	}
 }
+
+// TestIsolation_MarkGarminConnectedNeverLeaksAcrossUsers confirms marking
+// one user's Garmin connection never sets another user's flag.
+func TestIsolation_MarkGarminConnectedNeverLeaksAcrossUsers(t *testing.T) {
+	db := openTestDB(t)
+	ctx := context.Background()
+
+	userA, err := db.ProvisionUser(ctx, "sub-a", "A")
+	if err != nil {
+		t.Fatalf("ProvisionUser(a): %v", err)
+	}
+	userB, err := db.ProvisionUser(ctx, "sub-b", "B")
+	if err != nil {
+		t.Fatalf("ProvisionUser(b): %v", err)
+	}
+
+	if err := db.MarkGarminConnected(ctx, userA); err != nil {
+		t.Fatalf("MarkGarminConnected(a): %v", err)
+	}
+
+	profileB, err := db.GetProfile(ctx, userB)
+	if err != nil {
+		t.Fatalf("GetProfile(b): %v", err)
+	}
+	if profileB.GarminConnectedAt != nil {
+		t.Fatal("userA's MarkGarminConnected call leaked into userB's profile")
+	}
+}

backend/internal/store/profile.go

@@ -10,9 +10,15 @@ import (
 type Profile struct {
 	// Name labels this profile so a future multi-profile setup can show
 	// which one is active. Only one profile row exists today (id=1).
-	Name              string
+	Name           string
-	GarminEmail       string
+	GarminEmail    string
-	GarminPassword    string
+	GarminPassword string
+	// GarminConnectedAt is nil until this user's first successful Garmin
+	// authentication (see store.MarkGarminConnected) -- the login gate uses
+	// it, not UpdateProfile, so it's deliberately excluded from
+	// UpdateProfile's SET clause below and can only ever move from nil to
+	// set, never reset by a normal profile save.
+	GarminConnectedAt *string
 	RollingWindowDays int
 	// BackfillHorizonDays bounds how far back "Sync now" reaches when
 	// walking backward from today; it's read fresh on every sync (not fixed
@@ -70,7 +76,7 @@ type Profile struct {
 }
 
 const profileColumns = `
-	name, garmin_email, garmin_password, rolling_window_days, backfill_horizon_days, max_heart_rate, resting_heart_rate,
+	name, garmin_email, garmin_password, garmin_connected_at, rolling_window_days, backfill_horizon_days, max_heart_rate, resting_heart_rate,
 	hr_zone1_min_pct, hr_zone1_max_pct, hr_zone2_min_pct, hr_zone2_max_pct,
 	hr_zone3_min_pct, hr_zone3_max_pct, hr_zone4_min_pct, hr_zone4_max_pct,
 	hr_zone5_min_pct, hr_zone5_max_pct,
@@ -85,7 +91,7 @@ const profileColumns = `
 func (db *DB) GetProfile(ctx context.Context, userID int64) (Profile, error) {
 	var p Profile
 	err := db.QueryRowContext(ctx, `SELECT `+profileColumns+` FROM profile WHERE user_id = ?`, userID).Scan(
-		&p.Name, &p.GarminEmail, &p.GarminPassword, &p.RollingWindowDays, &p.BackfillHorizonDays, &p.MaxHeartRate, &p.RestingHeartRate,
+		&p.Name, &p.GarminEmail, &p.GarminPassword, &p.GarminConnectedAt, &p.RollingWindowDays, &p.BackfillHorizonDays, &p.MaxHeartRate, &p.RestingHeartRate,
 		&p.HRZone1MinPct, &p.HRZone1MaxPct, &p.HRZone2MinPct, &p.HRZone2MaxPct,
 		&p.HRZone3MinPct, &p.HRZone3MaxPct, &p.HRZone4MinPct, &p.HRZone4MaxPct,
 		&p.HRZone5MinPct, &p.HRZone5MaxPct,
@@ -132,3 +138,14 @@ func (db *DB) UpdateProfile(ctx context.Context, userID int64, p Profile) error
 	}
 	return nil
 }
+
+// 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 {
+	_, err := db.ExecContext(ctx, `UPDATE profile SET garmin_connected_at = datetime('now') WHERE user_id = ? AND garmin_connected_at IS NULL`, userID)
+	if err != nil {
+		return fmt.Errorf("mark garmin connected for user %d: %w", userID, err)
+	}
+	return nil
+}

backend/internal/store/profile_test.go

@@ -107,3 +107,44 @@ func TestProfile_DefaultsThenUpdate(t *testing.T) {
 		t.Errorf("TargetBrightenPct after update = %v, want 50", got.TargetBrightenPct)
 	}
 }
+
+func TestMarkGarminConnected_SetsTimestampOnceOnFirstCall(t *testing.T) {
+	db := openTestDB(t)
+	ctx := context.Background()
+	userID, err := db.ProvisionUser(ctx, "test-sub", "Test")
+	if err != nil {
+		t.Fatalf("ProvisionUser: %v", err)
+	}
+
+	before, err := db.GetProfile(ctx, userID)
+	if err != nil {
+		t.Fatalf("GetProfile: %v", err)
+	}
+	if before.GarminConnectedAt != nil {
+		t.Fatalf("expected a fresh profile to have nil GarminConnectedAt, got %v", *before.GarminConnectedAt)
+	}
+
+	if err := db.MarkGarminConnected(ctx, userID); err != nil {
+		t.Fatalf("MarkGarminConnected: %v", err)
+	}
+	afterFirst, err := db.GetProfile(ctx, userID)
+	if err != nil {
+		t.Fatalf("GetProfile after first mark: %v", err)
+	}
+	if afterFirst.GarminConnectedAt == nil {
+		t.Fatal("expected GarminConnectedAt to be set after MarkGarminConnected")
+	}
+	firstValue := *afterFirst.GarminConnectedAt
+
+	// A second call must not change the recorded first-connection time.
+	if err := db.MarkGarminConnected(ctx, userID); err != nil {
+		t.Fatalf("MarkGarminConnected (second call): %v", err)
+	}
+	afterSecond, err := db.GetProfile(ctx, userID)
+	if err != nil {
+		t.Fatalf("GetProfile after second mark: %v", err)
+	}
+	if afterSecond.GarminConnectedAt == nil || *afterSecond.GarminConnectedAt != firstValue {
+		t.Fatalf("GarminConnectedAt changed on second call: first=%q second=%v", firstValue, afterSecond.GarminConnectedAt)
+	}
+}

backend/internal/store/schema.sql

@@ -30,6 +30,12 @@ CREATE TABLE profile (
     name                TEXT NOT NULL DEFAULT 'Default',
     garmin_email        TEXT NOT NULL DEFAULT '',
     garmin_password     TEXT NOT NULL DEFAULT '',
+    -- 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,
     rolling_window_days INTEGER NOT NULL DEFAULT 90,
     -- BackfillHorizonDays bounds how far back "Sync now" reaches when
     -- walking backward from today; read fresh on every sync (not cached at

docs/DATABASE.md

@@ -35,6 +35,12 @@ CREATE TABLE profile (
     name                TEXT NOT NULL DEFAULT 'Default',
     garmin_email        TEXT NOT NULL DEFAULT '',
     garmin_password     TEXT NOT NULL DEFAULT '',
+    -- 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,
     rolling_window_days INTEGER NOT NULL DEFAULT 90,
     -- BackfillHorizonDays bounds how far back "Sync now" reaches when
     -- walking backward from today; read fresh on every sync (not cached at

docs/IDEAS.md

@@ -8,7 +8,6 @@ for that history) and remove it from here once a spec exists.
 
 ## Backlog
 
-- 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
 - dedicated workouts section: add workouts table in database and create a dedicated page in UI (use an "objective" icon) 
 - improve activities/workouts download: make it modal, add progress bar, error management
 - better UX when backend is not available (instead of "TypeError failed to fetch")

frontend/src/ConnectGarmin.tsx

@@ -0,0 +1,117 @@
+import { useState } from "react";
+import { api, BASE_URL } from "./api/client";
+import "./CreateProfile.css";
+import type { AuthResponse } from "./types/api";
+
+// Shown after CreateProfile (or on any later login, until it succeeds --
+// see LoginGate) since connecting Garmin is mandatory: the account exists
+// but this is the last gate before the main app. Deliberately not a reuse
+// of GarminConnection.tsx (the Profile page's version), which also renders
+// Sync now/Disconnect/Reset all -- none of which make sense here.
+export function ConnectGarmin({ onConnected }: { onConnected: () => void }) {
+  const [email, setEmail] = useState("");
+  const [password, setPassword] = useState("");
+  const [code, setCode] = useState("");
+  const [auth, setAuth] = useState<AuthResponse | null>(null);
+  const [busy, setBusy] = useState(false);
+  const [error, setError] = useState<string | null>(null);
+
+  async function connect(e: React.FormEvent) {
+    e.preventDefault();
+    if (!email.trim() || !password) {
+      setError("Please enter your Garmin email and password.");
+      return;
+    }
+    setBusy(true);
+    setError(null);
+    try {
+      const profile = await api.getProfile();
+      await api.updateProfile({ ...profile, GarminEmail: email.trim(), GarminPassword: password });
+      setAuth(await api.login());
+    } catch (err) {
+      setError(err instanceof Error ? err.message : "Something went wrong, please try again.");
+    } finally {
+      setBusy(false);
+    }
+  }
+
+  async function submitMFA() {
+    if (!code.trim()) return;
+    setBusy(true);
+    setError(null);
+    try {
+      setAuth(await api.submitMFA(code.trim()));
+      setCode("");
+    } catch (err) {
+      setError(err instanceof Error ? err.message : "Something went wrong, please try again.");
+    } finally {
+      setBusy(false);
+    }
+  }
+
+  const status = auth?.status;
+
+  return (
+    <div className="create-profile">
+      <h1>🧞‍♀️ Connect your Garmin account</h1>
+      <p>geniusrun needs your Garmin credentials to sync your activities.</p>
+
+      {status !== "authenticated" && status !== "mfa_required" && (
+        <form onSubmit={connect}>
+          <input
+            type="text"
+            placeholder="Garmin email"
+            value={email}
+            onChange={(e) => setEmail(e.target.value)}
+            disabled={busy}
+            autoFocus
+          />
+          <input
+            type="password"
+            placeholder="Garmin password"
+            value={password}
+            onChange={(e) => setPassword(e.target.value)}
+            disabled={busy}
+          />
+          <button type="submit" disabled={busy}>
+            {busy ? "Connecting…" : "Connect"}
+          </button>
+        </form>
+      )}
+
+      {status === "mfa_required" && (
+        <div className="create-profile-mfa">
+          <input
+            placeholder="MFA code"
+            value={code}
+            onChange={(e) => setCode(e.target.value)}
+            onKeyDown={(e) => e.key === "Enter" && submitMFA()}
+            disabled={busy}
+            autoFocus
+          />
+          <button type="button" disabled={busy} onClick={submitMFA}>
+            Submit code
+          </button>
+        </div>
+      )}
+
+      {status === "authenticated" && (
+        <>
+          <p className="create-profile-message">Connected to Garmin.</p>
+          <button type="button" onClick={onConnected}>
+            Continue to geniusrun
+          </button>
+        </>
+      )}
+
+      {auth?.message && status !== "authenticated" && <p className="create-profile-message">{auth.message}</p>}
+      {error && <p className="create-profile-error">{error}</p>}
+
+      <form method="post" action={`${BASE_URL}/api/session/logout`}>
+        <button type="submit" className="logout-link">
+          Log out
+        </button>
+      </form>
+    </div>
+  );
+}

frontend/src/CreateProfile.css

@@ -41,3 +41,16 @@
 .create-profile-error {
   color: #ef4444;
 }
+
+.create-profile-message {
+  margin: 0;
+  color: #9ca3af;
+}
+
+.create-profile-mfa {
+  display: flex;
+  flex-direction: column;
+  gap: 0.75rem;
+  width: 100%;
+  max-width: 320px;
+}

frontend/src/CreateProfile.tsx

@@ -1,11 +1,12 @@
 import { useState } from "react";
-import { api } from "./api/client";
+import { api, BASE_URL } from "./api/client";
 import "./CreateProfile.css";
 
 // Shown once, right after a brand-new OIDC login, before the account has a
-// geniusrun profile at all. The only thing asked for is a display name --
+// geniusrun profile at all. Only asks for a display name -- Garmin
-// Garmin credentials and every other tunable are filled in afterward via
+// credentials are collected next, on the ConnectGarmin screen (see
-// the existing Profile screen, same as a fresh single-user install today.
+// LoginGate), which every other tunable is still filled in afterward via
+// the existing Profile screen.
 export function CreateProfile({ onCreated }: { onCreated: (displayName: string) => void }) {
   const [displayName, setDisplayName] = useState("");
   const [submitting, setSubmitting] = useState(false);
@@ -32,7 +33,7 @@ export function CreateProfile({ onCreated }: { onCreated: (displayName: string)
   return (
     <div className="create-profile">
       <h1>🧞‍♀️ Welcome to geniusrun</h1>
-      <p>Let's set up your profile. You can add your Garmin account afterward.</p>
+      <p>Let's set up your profile. You'll connect your Garmin account next.</p>
       <form onSubmit={handleSubmit}>
         <input
           type="text"
@@ -47,6 +48,11 @@ export function CreateProfile({ onCreated }: { onCreated: (displayName: string)
           {submitting ? "Creating…" : "Continue"}
         </button>
       </form>
+      <form method="post" action={`${BASE_URL}/api/session/logout`}>
+        <button type="submit" className="logout-link">
+          Log out
+        </button>
+      </form>
     </div>
   );
 }

frontend/src/LoginGate.tsx

@@ -2,6 +2,7 @@ import { useEffect, useState } from "react";
 import { api, BASE_URL } from "./api/client";
 import "./LoginGate.css";
 import App from "./App";
+import { ConnectGarmin } from "./ConnectGarmin";
 import { CreateProfile } from "./CreateProfile";
 import type { SessionInfo } from "./types/api";
 
@@ -17,7 +18,10 @@ const AUTH_ERROR_MESSAGES: Record<string, string> = {
 // this is the first fork -- "show the login screen" vs "show the app." A
 // second fork, once authenticated, is whether the session's account has a
 // provisioned profile yet (session.has_profile) -- a brand-new OIDC login
-// sees CreateProfile instead of App until it submits one.
+// sees CreateProfile instead of App until it submits one. A third fork,
+// once provisioned, is whether Garmin has ever been successfully connected
+// (session.garmin_connected) -- mandatory, and re-checked on every login,
+// not just immediately after signup.
 export function LoginGate() {
   const [status, setStatus] = useState<Status>("loading");
   const [session, setSession] = useState<SessionInfo | null>(null);
@@ -57,5 +61,9 @@ export function LoginGate() {
     );
   }
 
+  if (!session!.garmin_connected) {
+    return <ConnectGarmin onConnected={() => setSession((s) => (s ? { ...s, garmin_connected: true } : s))} />;
+  }
+
   return <App session={session!} />;
 }

frontend/src/types/api.ts

@@ -191,6 +191,7 @@ export interface SessionInfo {
   email: string;
   has_profile: boolean;
   display_name?: string;
+  garmin_connected: boolean;
 }
 
 export interface DetailFillProgress {