From 0211dffa1eea0f737a54e41980c9662529b7f58b Mon Sep 17 00:00:00 2001 From: Christophe Vila Date: Sun, 26 Jul 2026 21:45:39 +0200 Subject: [PATCH] fix(config): stop deriving GarminTokenStoreRoot's default from DBPath There's no reason the Garmin token-store root and the SQLite database file need to live near each other -- they're independent env vars (GARMIN_TOKENSTORE, GENIUSRUN_DB_PATH) and should stay independently configurable, including in their defaults. Revert the "next to DBPath" default introduced in 77f9588 back to a plain ".garmin" relative to the working directory, so pointing GENIUSRUN_DB_PATH elsewhere never silently drags the token-store default along with it. --- CLAUDE.md | 2 +- backend/internal/config/config.go | 18 +++++++++--------- backend/internal/config/config_test.go | 8 +++----- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6c4025e..35dbd62 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -91,7 +91,7 @@ Each `workout_kinds.rule_json` is a recursive AND/OR condition tree (`internal/c - **`authenticate`/`complete_mfa` return structured JSON** (`{"status": "success"|"mfa_required"|"failed", "message": "..."}`), not plain strings — `internal/garmin/client.go` maps `status` directly to `AuthStatus`, no string pattern-matching. - **The 10s "MFA required" timeout in `wrapper.py`'s `authenticate` handler 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 (`wrapper.py` logs this to stderr for exactly this reason). -- **`wrapper.py` 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 is 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()` defaults it to a `.garmin` directory next to the DB file (logged at startup) rather than leaving per-user namespacing silently skipped. +- **`wrapper.py` 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 is 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()` defaults it to a `.garmin` directory relative to the working directory -- deliberately independent of `GENIUSRUN_DB_PATH`, so the DB file and the token-store root can each be pointed at their own directory -- 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. 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`). diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index f744375..b578a91 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -8,7 +8,6 @@ package config import ( "fmt" "os" - "path/filepath" "strconv" "strings" "time" @@ -27,11 +26,13 @@ type Config struct { GarminPythonPath string // GarminTokenStoreRoot is the root directory under which each user's // Garmin session cache lives (one subdirectory per user id, e.g. - // "/3"). Read from the GARMIN_TOKENSTORE env var; if unset, it - // defaults to a ".garmin" directory next to DBPath so every - // deployment gets per-user isolation automatically -- multi-tenant - // operation always relies on this being a real, distinct-per-user path - // (see api.Server.garminFor), so it can never be silently left empty. + // "/3"). Read from the GARMIN_TOKENSTORE env var, configured + // independently of DBPath -- if unset, it defaults to a ".garmin" + // directory relative to the working directory the process is started + // from, not derived from DBPath in any way, so every deployment gets + // per-user isolation automatically -- multi-tenant operation always + // relies on this being a real, distinct-per-user path (see + // api.Server.garminFor), so it can never be silently left empty. GarminTokenStoreRoot string MinConfidence float64 @@ -68,12 +69,11 @@ type Config struct { // Load reads configuration from environment variables, applying defaults // for anything optional. Returns an error if a required variable is unset. func Load() (Config, error) { - dbPath := getEnvDefault("GENIUSRUN_DB_PATH", "geniusrun.db") cfg := Config{ Addr: getEnvDefault("GENIUSRUN_ADDR", ":8080"), - DBPath: dbPath, + DBPath: getEnvDefault("GENIUSRUN_DB_PATH", "geniusrun.db"), GarminPythonPath: getEnvDefault("GARMIN_WRAPPER_PYTHON", "python3"), - GarminTokenStoreRoot: getEnvDefault("GARMIN_TOKENSTORE", filepath.Join(filepath.Dir(dbPath), ".garmin")), + GarminTokenStoreRoot: getEnvDefault("GARMIN_TOKENSTORE", ".garmin"), MinConfidence: getEnvFloat("GENIUSRUN_MIN_CONFIDENCE", 0.6), LogLevel: getEnvDefault("GENIUSRUN_LOG_LEVEL", "info"), BackendURL: strings.TrimRight(os.Getenv("GENIUSRUN_BACKEND_URL"), "/"), diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index 9e37447..2717af9 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "path/filepath" "testing" "time" ) @@ -79,7 +78,7 @@ func TestLoad_SessionSecretTooShort(t *testing.T) { } } -func TestLoad_GarminTokenStoreRootDefaultsNextToDBPath(t *testing.T) { +func TestLoad_GarminTokenStoreRootDefaultsIndependentlyOfDBPath(t *testing.T) { setRequiredEnv(t) t.Setenv("GARMIN_TOKENSTORE", "") t.Setenv("GENIUSRUN_DB_PATH", "/var/lib/geniusrun/geniusrun.db") @@ -88,9 +87,8 @@ func TestLoad_GarminTokenStoreRootDefaultsNextToDBPath(t *testing.T) { if err != nil { t.Fatalf("Load: %v", err) } - want := filepath.Join("/var/lib/geniusrun", ".garmin") - if cfg.GarminTokenStoreRoot != want { - t.Errorf("GarminTokenStoreRoot = %q, want %q", cfg.GarminTokenStoreRoot, want) + if cfg.GarminTokenStoreRoot != ".garmin" { + t.Errorf("GarminTokenStoreRoot = %q, want %q (never derived from DBPath's directory)", cfg.GarminTokenStoreRoot, ".garmin") } }