chore: wire up the new garmin.Config shape, refresh start.sh and CLAUDE.md
Removes the last references to garmin.Config.ServerPath and the MCP_GARMIN_* env vars now that the wrapper script is embedded in the binary; updates CLAUDE.md's mcp-garmin section to describe the direct wrapper instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
20
CLAUDE.md
20
CLAUDE.md
@@ -13,7 +13,7 @@ The "Training plan" tab (`frontend/src/pages/Plan.tsx`) is an intentional empty
|
|||||||
## Commands
|
## Commands
|
||||||
|
|
||||||
Backend (from `backend/`):
|
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 ./...`
|
- Build/vet: `go build ./...` && `go vet ./...`
|
||||||
- Format: `gofmt -l .` must report nothing before committing.
|
- Format: `gofmt -l .` must report nothing before committing.
|
||||||
- All tests: `go test ./...`
|
- All tests: `go test ./...`
|
||||||
@@ -46,7 +46,7 @@ backend/
|
|||||||
cmd/seedsample/ inserts synthetic data for frontend dev/demoing without live Garmin creds
|
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/dumpschema/ regenerates docs/DATABASE.md from the live schema -- run after editing schema.sql
|
||||||
cmd/mcpspike/ throwaway MCP-client spike, safe to delete
|
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/garmin/mock/ fake Client for tests
|
||||||
internal/classify/ pure rule engine (condition tree eval, scoring, interval detection, HR drift/recovery)
|
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
|
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.
|
- 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.
|
- **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": "<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 plain strings**, not structured JSON (`"Authenticated successfully."`, `"MFA required. ..."`, `"Authentication failed: ..."`). `internal/garmin/client.go`'s `parseAuthResult` pattern-matches these.
|
- **`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 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).
|
- **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).
|
||||||
- **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.
|
- **`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`).
|
- **`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_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_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_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.
|
- 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
|
## Data model conventions
|
||||||
|
|||||||
@@ -44,7 +44,6 @@ func main() {
|
|||||||
|
|
||||||
server := api.NewServer(db, garmin.NewClient, garmin.Config{
|
server := api.NewServer(db, garmin.NewClient, garmin.Config{
|
||||||
PythonPath: cfg.GarminPythonPath,
|
PythonPath: cfg.GarminPythonPath,
|
||||||
ServerPath: cfg.GarminServerPath,
|
|
||||||
TokenStorePath: cfg.GarminTokenStoreRoot,
|
TokenStorePath: cfg.GarminTokenStoreRoot,
|
||||||
}, appsync.Config{
|
}, appsync.Config{
|
||||||
MinConfidence: cfg.MinConfidence,
|
MinConfidence: cfg.MinConfidence,
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ type Server struct {
|
|||||||
// *mock.Client (see newTestServer in api_test.go).
|
// *mock.Client (see newTestServer in api_test.go).
|
||||||
GarminFactory func(garmin.Config) garmin.Client
|
GarminFactory func(garmin.Config) garmin.Client
|
||||||
// GarminBase holds the plumbing shared by every user's garmin.Config
|
// 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
|
// GarminEmail/GarminPassword/TokenStorePath vary per user, filled in by
|
||||||
// garminFor.
|
// garminFor.
|
||||||
GarminBase garmin.Config
|
GarminBase garmin.Config
|
||||||
|
|||||||
@@ -2,8 +2,9 @@
|
|||||||
set -euo pipefail
|
set -euo pipefail
|
||||||
cd "$(dirname "$0")"
|
cd "$(dirname "$0")"
|
||||||
|
|
||||||
export MCP_GARMIN_PYTHON="${MCP_GARMIN_PYTHON:-/Users/cvila/Dev/scm/scm.vilanet.fr/kriss/mcp-garmin/.venv/bin/python3}"
|
# GARMIN_WRAPPER_PYTHON is optional (config.Load() defaults it to "python3"
|
||||||
export MCP_GARMIN_SERVER="${MCP_GARMIN_SERVER:-/Users/cvila/Dev/scm/scm.vilanet.fr/kriss/mcp-garmin/server.py}"
|
# 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}"
|
export GENIUSRUN_DB_PATH="${GENIUSRUN_DB_PATH:-geniusrun.db}"
|
||||||
|
|
||||||
# OIDC login gate -- no safe default exists for a client secret or a
|
# OIDC login gate -- no safe default exists for a client secret or a
|
||||||
|
|||||||
Reference in New Issue
Block a user