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

@@ -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**