refactor(config): rename default Garmin token-store dir to .garmin

Shorter, more conventional dotdir name than garmin-tokenstores. Restore
the "next to DBPath" join that got dropped when the default was
inlined via getEnvDefault -- an explicit GARMIN_TOKENSTORE still wins
verbatim, but the implicit default must still resolve relative to the
DB file's directory, not the process's CWD.

internal/garmin/client.go's GARMIN_TOKENSTORE env var is now always
set (TokenStorePath can no longer be empty), so the conditional that
only appended it when non-empty is dead code.
This commit is contained in:
2026-07-26 21:25:39 +02:00
parent a12fe3e810
commit 77f9588c2d
5 changed files with 73 additions and 34 deletions

View File

@@ -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-tokenstores` 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 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. 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`).

View File

@@ -7,7 +7,6 @@ package config
import (
"fmt"
"log"
"os"
"path/filepath"
"strconv"
@@ -29,7 +28,7 @@ type Config struct {
// GarminTokenStoreRoot is the root directory under which each user's
// Garmin session cache lives (one subdirectory per user id, e.g.
// "<root>/3"). Read from the GARMIN_TOKENSTORE env var; if unset, it
// defaults to a "garmin-tokenstores" directory next to DBPath so every
// 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.
@@ -69,11 +68,12 @@ 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: getEnvDefault("GENIUSRUN_DB_PATH", "geniusrun.db"),
DBPath: dbPath,
GarminPythonPath: getEnvDefault("GARMIN_WRAPPER_PYTHON", "python3"),
GarminTokenStoreRoot: os.Getenv("GARMIN_TOKENSTORE"),
GarminTokenStoreRoot: getEnvDefault("GARMIN_TOKENSTORE", filepath.Join(filepath.Dir(dbPath), ".garmin")),
MinConfidence: getEnvFloat("GENIUSRUN_MIN_CONFIDENCE", 0.6),
LogLevel: getEnvDefault("GENIUSRUN_LOG_LEVEL", "info"),
BackendURL: strings.TrimRight(os.Getenv("GENIUSRUN_BACKEND_URL"), "/"),
@@ -84,15 +84,11 @@ func Load() (Config, error) {
SessionDuration: getEnvDuration("GENIUSRUN_SESSION_DURATION", 720*time.Hour),
}
if cfg.GarminTokenStoreRoot == "" {
cfg.GarminTokenStoreRoot = filepath.Join(filepath.Dir(cfg.DBPath), "garmin-tokenstores")
log.Printf("GARMIN_TOKENSTORE not set, defaulting to %q", cfg.GarminTokenStoreRoot)
}
if cfg.BackendURL == "" {
return cfg, fmt.Errorf("GENIUSRUN_BACKEND_URL is required (e.g. https://geniusrun.example.com)")
}
cfg.FrontendURL = strings.TrimRight(getEnvDefault("GENIUSRUN_FRONTEND_URL", cfg.BackendURL), "/")
if cfg.OIDCIssuerURL == "" {
return cfg, fmt.Errorf("GENIUSRUN_OIDC_ISSUER_URL is required")
}
@@ -102,12 +98,13 @@ func Load() (Config, error) {
if cfg.OIDCClientSecret == "" {
return cfg, fmt.Errorf("GENIUSRUN_OIDC_CLIENT_SECRET is required")
}
cfg.OIDCRedirectURL = cfg.BackendURL + "/api/session/callback"
sessionSecret := os.Getenv("GENIUSRUN_SESSION_SECRET")
if len(sessionSecret) < 32 {
return cfg, fmt.Errorf("GENIUSRUN_SESSION_SECRET is required and must be at least 32 characters")
}
cfg.SessionSecret = []byte(sessionSecret)
cfg.OIDCRedirectURL = cfg.BackendURL + "/api/session/callback"
cfg.SessionSecure = strings.HasPrefix(cfg.BackendURL, "https://")
return cfg, nil

View File

@@ -88,7 +88,7 @@ func TestLoad_GarminTokenStoreRootDefaultsNextToDBPath(t *testing.T) {
if err != nil {
t.Fatalf("Load: %v", err)
}
want := filepath.Join("/var/lib/geniusrun", "garmin-tokenstores")
want := filepath.Join("/var/lib/geniusrun", ".garmin")
if cfg.GarminTokenStoreRoot != want {
t.Errorf("GarminTokenStoreRoot = %q, want %q", cfg.GarminTokenStoreRoot, want)
}

View File

@@ -63,8 +63,8 @@ type Config struct {
PythonPath string
GarminEmail string
GarminPassword string
// TokenStorePath, if set, is passed as GARMIN_TOKENSTORE so the wrapper
// persists/resumes a Garmin session there instead of the default ~/.garth.
// TokenStorePath is passed as GARMIN_TOKENSTORE so the wrapper
// persists/resumes a Garmin session there.
TokenStorePath string
}
@@ -155,11 +155,9 @@ func (c *subprocessClient) ensureStarted(ctx context.Context) error {
extraEnv := []string{
"GARMIN_EMAIL=" + c.cfg.GarminEmail,
"GARMIN_PASSWORD=" + c.cfg.GarminPassword,
"GARMIN_TOKENSTORE=" + c.cfg.TokenStorePath,
"PYTHONUNBUFFERED=1",
}
if c.cfg.TokenStorePath != "" {
extraEnv = append(extraEnv, "GARMIN_TOKENSTORE="+c.cfg.TokenStorePath)
}
cmd.Env = append(os.Environ(), extraEnv...)
stdin, err := cmd.StdinPipe()
@@ -199,7 +197,6 @@ func (c *subprocessClient) ensureStarted(ctx context.Context) error {
applog.FromContext(ctx).Info("garmin wrapper spawning",
"python_path", pythonPath,
"token_store_configured", c.cfg.TokenStorePath != "",
)
return nil
}

View File

@@ -1596,16 +1596,37 @@ Replace the entire `## mcp-garmin integration` section:
```markdown
## mcp-garmin integration
`internal/garmin` is a Go MCP **client** (via `github.com/mark3labs/mcp-go`'s stdio transport) that spawns `mcp-garmin`'s `server.py` as a subprocess. Key things learned the hard way, that would otherwise get rediscovered:
`internal/garmin` is a Go MCP **client** (via `github.com/mark3labs/mcp-go`'s stdio transport) that spawns `mcp-garmin`
's `server.py` as a subprocess. Key things learned the hard way, that would otherwise get rediscovered:
- **`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 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.
- **`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 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` 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.
- 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.
- **`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.
- 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.
```
with:
@@ -1613,16 +1634,40 @@ with:
```markdown
## Garmin integration (direct wrapper, no MCP)
`internal/garmin` spawns a small Python wrapper script (`internal/garmin/pyscript/wrapper.py`, embedded into the Go binary via `go:embed` — see `docs/superpowers/specs/2026-07-25-garmin-direct-wrapper-design.md`) that imports `garminconnect` directly and speaks a minimal newline-delimited-JSON protocol over stdin/stdout: `{"id","cmd","params"}` in, `{"id","result"}` or `{"id","error"}` out. `cmd` is `authenticate`, `complete_mfa`, or a fully generic `call` (`{"method": "<any garminconnect.Garmin method>", "args": {...}}`) — there's no MCP layer, and the separate `mcp-garmin` repo this used to depend on is retired. Key things learned the hard way, that would otherwise get rediscovered:
`internal/garmin` spawns a small Python wrapper script (`internal/garmin/pyscript/wrapper.py`, embedded into the Go
binary via `go:embed` — see `docs/superpowers/specs/2026-07-25-garmin-direct-wrapper-design.md`) that imports
`garminconnect` directly and speaks a minimal newline-delimited-JSON protocol over stdin/stdout: `{"id","cmd","params"}`
in, `{"id","result"}` or `{"id","error"}` out. `cmd` is `authenticate`, `complete_mfa`, or a fully generic `call`
(`{"method": "<any garminconnect.Garmin method>", "args": {...}}`) — there's no MCP layer, and the separate `mcp-garmin`
repo this used to depend on is retired. Key things learned the hard way, that would otherwise get rediscovered:
- **`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-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. 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.
- **`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.
- **`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`).
- **`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.
- 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.
- **`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.
- 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.
```
- [ ] **Step 5: Verify the whole backend builds**