diff --git a/CLAUDE.md b/CLAUDE.md index f61db39..6965976 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,7 @@ The "Training plan" tab (`frontend/src/pages/Plan.tsx`) is an intentional empty ## Commands Backend (from `backend/`): -- Run the server: `./start.sh` (wraps `go run ./cmd/geniusrund` with the local mcp-garmin subprocess paths and DB path already set) or `go run ./cmd/geniusrund` directly if you export `MCP_GARMIN_PYTHON`/`MCP_GARMIN_SERVER` yourself. +- Run the server: `./start.sh` (wraps `go run ./cmd/geniusrund` with the DB path already set) or `go run ./cmd/geniusrund` directly. The Garmin wrapper's Python interpreter defaults to `python3` on `PATH`; set `GARMIN_WRAPPER_PYTHON` if you need a specific one (e.g. a venv with `garminconnect` installed). - Build/vet: `go build ./...` && `go vet ./...` - Format: `gofmt -l .` must report nothing before committing. - All tests: `go test ./...` @@ -46,7 +46,7 @@ backend/ cmd/seedsample/ inserts synthetic data for frontend dev/demoing without live Garmin creds cmd/dumpschema/ regenerates docs/DATABASE.md from the live schema -- run after editing schema.sql cmd/mcpspike/ throwaway MCP-client spike, safe to delete - internal/garmin/ MCP client wrapper (auth, get_activities, get_activity_splits, get_activity_details, get_workout_by_id) + internal/garmin/ Garmin Connect client: spawns an embedded Python wrapper (pyscript/wrapper.py, garminconnect) over a JSON-lines subprocess protocol (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; schema.sql is the whole schema (no migration history), users.go owns account provisioning @@ -86,17 +86,17 @@ Each `workout_kinds.rule_json` is a recursive AND/OR condition tree (`internal/c - Max HR for `avg_hr_pct_max` comes from `profile.max_heart_rate`, not a static config value — nil/unset omits that metric from the context rather than erroring. - **Race** is special-cased: `is_race` is derived at sync time from Garmin's own `eventType.typeKey == "race"` (see `internal/sync/mapping.go`), not a rule the user tunes. Once an activity is assigned Race (or manually overridden to any kind), `POST /api/reclassify` (`handleReclassifyAll`) skips it — Race is a hard Garmin fact and manual assignments are the user's definitive word, neither is ever silently overwritten by a global reclassify. -## mcp-garmin integration +## Garmin integration (direct wrapper, no MCP) -`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` 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": "", "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 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. +- **`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, 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. +- **`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. ## Data model conventions diff --git a/backend/cmd/geniusrund/main.go b/backend/cmd/geniusrund/main.go index 318b8ee..aec7434 100644 --- a/backend/cmd/geniusrund/main.go +++ b/backend/cmd/geniusrund/main.go @@ -44,7 +44,6 @@ func main() { server := api.NewServer(db, garmin.NewClient, garmin.Config{ PythonPath: cfg.GarminPythonPath, - ServerPath: cfg.GarminServerPath, TokenStorePath: cfg.GarminTokenStoreRoot, }, appsync.Config{ MinConfidence: cfg.MinConfidence, diff --git a/backend/internal/api/server.go b/backend/internal/api/server.go index 5bf5f83..9798bf4 100644 --- a/backend/internal/api/server.go +++ b/backend/internal/api/server.go @@ -34,7 +34,7 @@ type Server struct { // *mock.Client (see newTestServer in api_test.go). GarminFactory func(garmin.Config) garmin.Client // GarminBase holds the plumbing shared by every user's garmin.Config - // (subprocess paths + the token-store root directory); only + // (the python interpreter path + the token-store root directory); only // GarminEmail/GarminPassword/TokenStorePath vary per user, filled in by // garminFor. GarminBase garmin.Config diff --git a/backend/start.sh b/backend/start.sh index e67b1c8..d96fc99 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -2,8 +2,9 @@ set -euo pipefail cd "$(dirname "$0")" -export MCP_GARMIN_PYTHON="${MCP_GARMIN_PYTHON:-/Users/cvila/Dev/scm/scm.vilanet.fr/kriss/mcp-garmin/.venv/bin/python3}" -export MCP_GARMIN_SERVER="${MCP_GARMIN_SERVER:-/Users/cvila/Dev/scm/scm.vilanet.fr/kriss/mcp-garmin/server.py}" +# GARMIN_WRAPPER_PYTHON is optional (config.Load() defaults it to "python3" +# on PATH). Export it yourself only if you want a specific interpreter, e.g. +# a local venv with garminconnect installed. export GENIUSRUN_DB_PATH="${GENIUSRUN_DB_PATH:-geniusrun.db}" # OIDC login gate -- no safe default exists for a client secret or a