From 15c2b6a2b66a02b67f3dd8a5da1ef18886fb3441 Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Sat, 25 Jul 2026 19:05:09 +0200 Subject: [PATCH] docs: update CLAUDE.md for per-user profile isolation Reflects the just-merged per-user-profile work: every table is now user_id-scoped, OIDC accounts get their own isolated dataset via resolveUser/requireProvisionedUser + POST /api/setup, Garmin sessions are namespaced per user, and the legacy-owner upgrade bootstrap. Supersedes the old "single shared profile" framing throughout. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8f6a95c..d079263 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co geniusrun (Go module `geniusrun/backend`) is a personal web app that pulls running activities from Garmin Connect (via the `mcp-garmin` MCP server), classifies each run into one of a fixed set of running-specific "workout kinds" (Easy, Long, 60' Threshold, 30' Threshold, Tempo, Intervals, MAS Test, Race), and charts progression over time per kind. Ambiguous runs (matching zero, multiple, or only weakly one kind) go to a manual review queue instead of being silently misclassified. -All Garmin credentials and every tunable analysis-engine parameter (HR zones, phase-detection minutes, pace-artifact filtering, chart colors, etc.) live in a single-row `profile` table, edited from the frontend's Profile screen — there is no multi-profile support, and `internal/config`'s env vars are limited to process-level plumbing (listen addr, DB path, mcp-garmin subprocess paths). +All Garmin credentials and every tunable analysis-engine parameter (HR zones, phase-detection minutes, pace-artifact filtering, chart colors, etc.) live in a `profile` row scoped to the logged-in user, edited from the frontend's Profile screen. Every table holding synced/tunable data (`profile`, `workout_kinds`/`workout_type_paces`, `activities` and everything under it, `sync_state`/`sync_runs`) is scoped to a `user_id`, so each OIDC account gets its own fully isolated dataset — see Authentication below. There's no admin UI or profile switcher, and `internal/config`'s env vars are limited to process-level plumbing (listen addr, DB path, mcp-garmin subprocess paths). The "Training plan" tab (`frontend/src/pages/Plan.tsx`) is an intentional empty stub — a future training-recommendation engine (analyzing training-effect balance across kinds, and a pace/HR-zone "delta" between declared targets and workout-derived reality) is deferred; see `docs/superpowers/specs/` and `docs/superpowers/plans/` for the design history behind what's already built and what's still open. @@ -28,11 +28,17 @@ Frontend (from `frontend/`): - Lint: `npm run lint` (oxlint) - No frontend test suite exists yet. -## Authentication +## Authentication & per-user data model -geniusrun requires login via an existing Keycloak realm (OIDC Authorization Code flow, backend-driven -- the browser never sees a Keycloak token, only geniusrun's own signed session cookie). This is an access gate only: every authenticated+authorized user reaches the same single profile/dataset described above, there is no per-user data scoping. Access additionally requires a specific realm role (`GENIUSRUN_OIDC_REQUIRED_ROLE`, default `geniusrun-user`) -- a successful Keycloak login alone is not sufficient if the realm is shared with other apps/users. +geniusrun requires login via an existing Keycloak realm (OIDC Authorization Code flow, backend-driven -- the browser never sees a Keycloak token, only geniusrun's own signed session cookie). Access additionally requires a specific realm role (`GENIUSRUN_OIDC_REQUIRED_ROLE`, default `geniusrun-user`) -- a successful Keycloak login alone is not sufficient if the realm is shared with other apps/users. -Required env vars (`config.Load()` fails fast if any are unset, same pattern as the Garmin paths above): `GENIUSRUN_PUBLIC_BASE_URL`, `GENIUSRUN_OIDC_ISSUER_URL`, `GENIUSRUN_OIDC_CLIENT_ID`, `GENIUSRUN_OIDC_CLIENT_SECRET`, `GENIUSRUN_SESSION_SECRET` (>=32 characters). Optional: `GENIUSRUN_OIDC_REQUIRED_ROLE`, `GENIUSRUN_SESSION_DURATION` (default `720h`). See `docs/superpowers/specs/2026-07-24-oidc-authentication-design.md` for the full design and `internal/auth` for the implementation. +Beyond the login gate, every authenticated+authorized OIDC subject maps 1:1 to its own geniusrun `users` row and its own fully isolated dataset — there is no shared profile/dataset across accounts, no admin UI, and no profile switcher. `internal/api/usercontext.go`'s `resolveUser` middleware resolves the session's OIDC subject to a `store.User` (never blocking by itself); `requireProvisionedUser` 403s every data route until one exists. Every handler derives `userID` **only** from `userIDFromContext(r.Context())` — never a URL param, query string, or request body field; this is the entire security boundary the isolation depends on. See `docs/superpowers/plans/2026-07-25-per-user-profile.md` for the full design/implementation history, including the store/API-layer scoping pattern and the bugs it took to get right (a mid-transaction SQLite `PRAGMA foreign_keys` no-op, and a chi middleware-ordering panic). + +A brand-new OIDC subject with no `users` row yet is routed to a "Create your profile" screen (`frontend/src/CreateProfile.tsx`) instead of the app. `POST /api/setup` (display name only) provisions it — a `users` row, a default `profile` row, the 8-kind workout taxonomy, and an initial `sync_state` row, all in one transaction (`store.ProvisionUser`). Garmin credentials, HR zones, etc. are filled in afterward via the normal Profile screen, same as any fresh install. + +Upgrading a pre-per-user-profile install (binding the previously-singleton `profile`/`activities`/`workout_kinds`/`sync_state`/`sync_runs` rows to one real account) is a one-time step: set `GENIUSRUN_LEGACY_OWNER_OIDC_SUB` to that account's OIDC subject before the first startup after upgrading — `store.ClaimLegacyOwner` runs once at boot, binds every row still marked `user_id IS NULL`, and is a permanent no-op afterward (safe to leave the env var set indefinitely). + +Required env vars (`config.Load()` fails fast if any are unset, same pattern as the Garmin paths above): `GENIUSRUN_PUBLIC_BASE_URL`, `GENIUSRUN_OIDC_ISSUER_URL`, `GENIUSRUN_OIDC_CLIENT_ID`, `GENIUSRUN_OIDC_CLIENT_SECRET`, `GENIUSRUN_SESSION_SECRET` (>=32 characters). Optional: `GENIUSRUN_OIDC_REQUIRED_ROLE`, `GENIUSRUN_SESSION_DURATION` (default `720h`), `GENIUSRUN_LEGACY_OWNER_OIDC_SUB` (one-time upgrade bootstrap, see above). See `docs/superpowers/specs/2026-07-24-oidc-authentication-design.md` for the original login-gate design and `internal/auth` for its implementation; the per-user data model on top of it lives in `internal/store` (schema/queries) and `internal/api/usercontext.go`+`setup.go` (HTTP layer). ## Repo layout @@ -44,12 +50,13 @@ backend/ internal/garmin/ MCP client wrapper (auth, get_activities, get_activity_splits, get_activity_details, get_workout_by_id) internal/garmin/mock/ fake Client for tests internal/classify/ pure rule engine (condition tree eval, scoring, interval detection, HR drift/recovery) - internal/store/ SQLite layer + embedded migrations - internal/sync/ orchestrates fetch -> store -> classify - internal/api/ HTTP handlers (chi router) + internal/store/ SQLite layer + embedded migrations; users.go/legacy_claim.go own account provisioning + the one-time upgrade bootstrap + internal/sync/ orchestrates fetch -> store -> classify; one Service instance per user + internal/api/ HTTP handlers (chi router); usercontext.go/setup.go own the per-user access boundary internal/config/ env var config loading (process-level only, not user-tunable params) frontend/ src/pages/ ReviewQueue ("Activities" tab), Dashboard ("Progression" tab), Plan (stub), Profile (reached via the profile-name button, not a tab) + src/CreateProfile.tsx "Create your profile" screen shown to a brand-new, unprovisioned OIDC session src/components/charts/ Recharts wrappers (ExpectedVsActualChart, ProgressionChart) src/components/ ColorField, PaceField, NullableNumberField, RawDataModal, TrainingTypesCard src/api/client.ts thin typed fetch client @@ -86,33 +93,35 @@ Each `workout_kinds.rule_json` is a recursive AND/OR condition tree (`internal/c - **`authenticate()`/`complete_mfa()` return plain strings**, not structured JSON (`"Authenticated successfully."`, `"MFA required. ..."`, `"Authentication failed: ..."`). `internal/garmin/client.go`'s `parseAuthResult` pattern-matches these. - **The 10s "MFA required" timeout in mcp-garmin's `authenticate()` is a false-positive trap.** A login that's merely slow (e.g. Garmin/Cloudflare rate-limiting) looks identical to a real MFA challenge unless you check whether `prompt_mfa()` was actually invoked (mcp-garmin now logs this to stderr for exactly this reason). -- **mcp-garmin persists Garmin sessions** via a `GARMIN_TOKENSTORE` env var (default `~/.garth`) passed to `Garmin.login(tokenstore=...)` — without this, every process start does a full SSO login, which is what trips Garmin's rate limiting under repeated testing. +- **mcp-garmin persists Garmin sessions** via a `GARMIN_TOKENSTORE` env var passed to `Garmin.login(tokenstore=...)` — without this, every process start does a full SSO login, which is what trips Garmin's rate limiting under repeated testing. Since geniusrun became multi-tenant, `GARMIN_TOKENSTORE` (`config.GarminTokenStoreRoot`) is a **root directory**, not a single session path: `api.Server.garminFor` namespaces each user's subprocess under `{root}/{userID}` so concurrent users never share a Garmin session. If unset, `config.Load()` now defaults it to a `garmin-tokenstores` directory next to the DB file (logged at startup) rather than leaving per-user namespacing silently skipped. - **`activityId` is a large int64** — never round-trip it through `float64`/generic `map[string]any` JSON decoding, or it corrupts into scientific notation. Decode into typed structs (`garmin.Activity`, not `map[string]any`). - **`get_activity_details()` returns raw per-second telemetry** (`activityDetailMetrics` + `metricDescriptors`), not lap/split summaries, despite what its docstring used to say. Its `metricDescriptors` index-to-field mapping is **not stable across activities/devices** — `garmin.ExtractSamples` always resolves fields by descriptor key, never by fixed array position. - **`get_activity_splits()`** returns the actual lap/split summaries (`lapDTOs`). - **`get_workout_by_id()`** returns a structured Garmin workout's flattened steps (target pace/HR per step); `internal/sync/mapping.go`'s `alignWorkoutTargets` zips these 1:1 against an activity's recorded laps (when the activity carries a `workout_id` and the lap count matches, tolerating exactly one extra trailing lap) to resolve each lap's expected pace/HR band. -- Garmin credentials are set via `garmin.Client.UpdateCredentials` when the profile is saved, which resets the client's "started" state and terminates any already-spawned subprocess so the next call respawns fresh under the new credentials — no separate reconnect UI needed beyond the existing connect/MFA flow. +- Each user has their own lazily-built, cached `garmin.Client` (`api.Server.garminFor`, double-checked-locked, keyed by `userID`). Saving that user's profile calls `UpdateCredentials` on their own cached client instance, which resets only that client's "started" state and terminates only that user's already-spawned subprocess so the next call respawns fresh under the new credentials — no separate reconnect UI needed beyond the existing connect/MFA flow, and no risk of one user's credential change touching another's session. ## Data model conventions +- **Every table is scoped to a `user_id`, one way or another.** `users` (keyed by OIDC subject) anchors it. `profile`, `workout_kinds`, `activities`, `sync_state`, `sync_runs` each carry their own `user_id` column and are queried with an explicit `WHERE user_id = ?`. `laps`, `activity_samples`, `kind_assignments`, and `workout_type_paces` have **no `user_id` column of their own** — they're always accessed through a specific owning row (an activity or a workout kind) via a `JOIN`/subquery back to that row's `user_id`, since they're never queried except through that owner. Every `internal/store` method that touches any of this takes an explicit `userID` parameter used in a real `WHERE`/`JOIN` clause — accepting the parameter without using it to filter would be a real cross-user leak, not a style nit. - `kind_assignments` is **append-only** — always INSERT, never UPDATE. Re-classifying after a rule edit, or a manual override, keeps full history; `current_kind_assignment` (a view) picks the latest row per activity by `id`. `assignment_source` distinguishes `manual` overrides (locked against future global reclassifies) from rule-engine assignments. - **Raw-JSON is the source of truth for anything not actively computed on.** `activities.raw_json`/`details_raw_json`/`workout_raw_json` store the full original Garmin JSON. Fields that are a pure untransformed copy of something already in `raw_json` (e.g. `activity_name`, `activity_type`, lap `duration_seconds`/`avg_hr`) were dropped from their own columns entirely (migration `0013_dedup_activity_lap_columns.sql`) and are instead decoded fresh at API-response time by `internal/api/display_fields.go` (`decodeActivityDisplayFields`/`decodeLapDisplayFields`) — don't reintroduce a stored column for something derivable from `raw_json` alone. -- `garmin_activity_id` is the natural idempotency key for `UpsertActivity` (`ON CONFLICT ... DO UPDATE`), safe to re-run on every sync pass. -- `sync_state` (singleton row) tracks a **backfill watermark** (`earliest_synced_date`, `backfill_complete`) — since Garmin history is immutable once recorded, `Service.Backfill` uses this to resume from where it left off (or no-op entirely if the configured horizon is already fully covered). `Backfill` reads `Profile.BackfillHorizonDays` fresh on every call (not a fixed `Config` field), so widening it in the Profile page takes effect on the very next sync, no restart needed. "Sync now" (`POST /api/sync/run`) calls `Backfill` then `IncrementalSync` then `FillPendingDetails` in one pass — there's no separate "full backfill" trigger. "Reset all" (`POST /api/sync/reset`) deletes every activity (cascading to laps/samples/kind_assignments) and rewinds the watermark — the only way to get already-synced activities re-processed against newer schema fields, since `FillPendingDetails` only ever touches activities whose details were never fetched. -- Live sync progress is exposed via `Service.Progress()` (in-memory, mutex-guarded `Done`/`Total` counters, reset to zero when idle) and surfaced through `GET /api/sync/status`. The frontend's `GarminConnection` banner polls this and shows "syncing: N/M activities" live. -- The 8-type taxonomy (`workout_kinds`) is a fixed, closed set with a fixed display order (`priority DESC, name`) — there's no create/delete UI, only rule/pace/HR-zone/color editing per type. `workout_type_paces` holds each type's target pace range + expected HR zone, informational only (never read by the classification engine) — no history, overwritten in place, since the synced activity log is the history. +- `(user_id, garmin_activity_id)` is the natural idempotency key for `UpsertActivity` (`ON CONFLICT ... DO UPDATE`), safe to re-run on every sync pass — scoped by user so two different users' Garmin accounts can never collide even if their activity IDs coincided. +- `sync_state` (one row per user) tracks a **backfill watermark** (`earliest_synced_date`, `backfill_complete`) — since Garmin history is immutable once recorded, `Service.Backfill` uses this to resume from where it left off (or no-op entirely if the configured horizon is already fully covered). `Backfill` reads `Profile.BackfillHorizonDays` fresh on every call (not a fixed `Config` field), so widening it in the Profile page takes effect on the very next sync, no restart needed. "Sync now" (`POST /api/sync/run`) calls `Backfill` then `IncrementalSync` then `FillPendingDetails` in one pass — there's no separate "full backfill" trigger. "Reset all" (`POST /api/sync/reset`) deletes every activity **belonging to that user** (cascading to laps/samples/kind_assignments) and rewinds their watermark — the only way to get already-synced activities re-processed against newer schema fields, since `FillPendingDetails` only ever touches activities whose details were never fetched. +- Live sync progress is exposed via `Service.Progress()` (in-memory, mutex-guarded `Done`/`Total` counters, reset to zero when idle) and surfaced through `GET /api/sync/status`. Since `sync.Service` is one instance per user, this is naturally per-user too. The frontend's `GarminConnection` banner polls this and shows "syncing: N/M activities" live. +- The 8-type taxonomy (`workout_kinds`) is a fixed, closed set with a fixed display order (`priority DESC, name`) — there's no create/delete UI, only rule/pace/HR-zone/color editing per type. Each user gets their own independently-tunable copy of the same 8 kinds, seeded by `store.ProvisionUser` on signup (`UNIQUE(user_id, name)`, not a global `UNIQUE(name)` — the same kind name is expected across different users). `workout_type_paces` holds each type's target pace range + expected HR zone, informational only (never read by the classification engine) — no history, overwritten in place, since the synced activity log is the history. - Chart colors (pace/HR line colors, warmup/effort/recovery/cooldown phase colors) are user-editable `profile` columns, not hardcoded, consumed by `ExpectedVsActualChart`. - Pace-artifact filtering (`profile.min_representative_pace_sec_per_km`/`min_representative_time_seconds`) drops brief slow-pace blips (GPS/motion settling at recording start) from the Review Queue chart unless they persist long enough to be a real stop/walk break. ## Dev workflow - `internal/garmin/mock` provides a fake `Client` for tests that need to exercise `internal/sync`/`internal/api` without a live subprocess. -- No live Garmin account needed for frontend/UI work: `cmd/seedsample` seeds realistic activities/laps/kinds through the real classification engine. +- No live Garmin account needed for frontend/UI work: `cmd/seedsample` provisions one fixed `"seedsample-user"` account (via `store.ProvisionUser`) then seeds realistic activities/laps/kinds under it through the real classification engine. ## Testing conventions - Table-driven Go tests throughout; test cases are inline, no separate fixture files. - `internal/classify` tests are pure (no DB/network): construct a `MetricContext` + `RuleKind`s directly. -- `internal/store` and `internal/sync` tests open a real temp-file SQLite DB (`store.Open` against `t.TempDir()`) — deliberate, not mocked, since migration/SQL correctness is exactly what needs catching. -- `internal/api` tests use `httptest` against a `Server` wired to a temp DB + `mock.Client`. +- `internal/store` and `internal/sync` tests open a real temp-file SQLite DB (`store.Open` against `t.TempDir()`) — deliberate, not mocked, since migration/SQL correctness is exactly what needs catching. Since every table is `user_id`-scoped, tests provision a user first (`db.ProvisionUser`) and thread that `userID` into every call. +- `internal/api` tests use `httptest` against a `Server` wired to a temp DB + `mock.Client`; `newTestServer` auto-provisions a `"test-user"` account matching the session cookie `doJSON` mints, so most handler tests don't need to think about provisioning at all. Tests that specifically need an *unprovisioned* session (e.g. the setup flow, or `resolveUser`'s not-found path) build a bare `NewServer` directly instead of using `newTestServer`. +- **Cross-user isolation is tested adversarially, not just in parallel** — `internal/store/isolation_test.go` and `internal/api/isolation_test.go` provision two real users and attempt real reads/writes against the *other* user's real row IDs, asserting on not-found/403/empty-result rather than merely checking two separately-created rows don't collide. Any new per-user-scoped feature should get the same treatment, not just a "two users each see their own data" happy-path test. - The MCP/Garmin integration itself can't be safely automated (real account, MFA, rate limits) — it's a manual smoke test via `cmd/mcpspike` or the real `geniusrund` auth endpoints.