Since wrapper.py now lives inside this repo, its location is no longer a deploy-time concern -- go:embed it into the binary and drop the GARMIN_WRAPPER_SCRIPT env var entirely. Only the Python interpreter choice remains configurable, and now optionally so (defaults to "python3"). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
13 KiB
Drop MCP for Garmin integration — direct Python wrapper design
Date: 2026-07-25
Overview
internal/garmin currently talks to mcp-garmin (a separate repo) as an MCP client over stdio, spawning server.py as a subprocess and driving it through JSON-RPC tool calls (authenticate, complete_mfa, get_activities, get_activity_splits, get_activity_details, get_workout_by_id). MCP's actual value — letting an LLM dynamically discover and choose among tools — is unused here: geniusrun's Go backend calls a fixed, hardcoded set of operations in fixed code paths. The MCP protocol layer (session init, capability negotiation, JSON-RPC envelope, the mcp-go client dependency in Go, the mcp SDK dependency in Python) buys nothing for this use case.
Both mcp-garmin and geniusrun are projects the same person owns, so there's no third-party maintenance benefit to keeping mcp-garmin as a separate abstraction either. Given a plan to package the backend in a container, fewer runtime dependencies is a concrete win: dropping the MCP layer means one fewer Python package (mcp[cli]) and one fewer Go dependency (github.com/mark3labs/mcp-go), and folding the wrapper script into the geniusrun repo itself means the container image doesn't need to vendor/clone a second repo.
This replaces the MCP transport with a small custom Python subprocess wrapper around garminconnect directly, speaking a minimal newline-delimited JSON protocol over stdin/stdout. mcp-garmin is retired once this ships.
Goals
- Remove the MCP protocol layer and its two SDK dependencies (
mcp-goin Go,mcp[cli]in Python) entirely. - Fold the Python wrapper into the geniusrun repo (
backend/garmin-wrapper/wrapper.py) so the container image only needs a Python runtime +pip install garminconnect+ this one file — no separate repo to fetch at build time. - Preserve
internal/garmin.Client's existing interface and behavior exactly (lazy subprocess spawn, per-user token store,UpdateCredentialstear-down/respawn, stderr draining) sointernal/syncandinternal/apirequire zero changes. - Replace today's fragile string pattern-matching of auth replies (
parseAuthResult, documented in CLAUDE.md as a "learned the hard way" wart) with structured JSON status, now that both sides of the protocol are under our control. - Make every method
python-garminconnect'sGarminclass exposes reachable from Go without per-method Python boilerplate, anticipating future AI-driven features (recovery/HRV context for training-plan recommendations) that may need data beyond what's synced today.
Non-goals
- No change to what data geniusrun actually syncs/stores/classifies today — only the transport between Go and Python changes.
internal/sync,internal/classify,internal/store, and the frontend are untouched. - No security sandboxing of the generic dispatcher (see below) — the subprocess is spoken to only by geniusrun's own Go process over a private pipe it owns, not exposed to any other caller, so an open dispatch surface is an accepted risk, not a gap.
- No auto-restart-on-crash logic for the subprocess. Matches today's behavior: a dead subprocess surfaces as a Go error on next use; only
UpdateCredentialsexplicitly tears down and respawns. - No change to the per-user token store layout (
{GarminTokenStoreRoot}/{userID}) or the multi-tenantgarminForcaching inapi.Server.
Wire protocol
Newline-delimited JSON, one object per line each direction, over the subprocess's stdin/stdout:
Go -> Python: {"id": 1, "cmd": "authenticate"}
Python -> Go: {"id": 1, "result": {"status": "success", "message": "Authenticated successfully."}}
Go -> Python: {"id": 2, "cmd": "complete_mfa", "params": {"code": "123456"}}
Python -> Go: {"id": 2, "result": {"status": "mfa_required", "message": "..."}}
Go -> Python: {"id": 3, "cmd": "call", "params": {"method": "get_activities_by_date", "args": {"start_date": "2026-07-01", "end_date": "2026-07-25"}}}
Python -> Go: {"id": 3, "result": [...]}
Go -> Python: {"id": 4, "cmd": "call", "params": {"method": "get_activity_splits", "args": {"activity_id": "123"}}}
Python -> Go: {"id": 4, "error": "Error fetching activity splits: <exception message>"}
Three command kinds:
authenticate/complete_mfa— special-cased in Python: same background-thread login + MFA-prompt race + 10-second timeout detection logicmcp-garmin'sserver.pyalready has (this is genuine custom control flow, not a passthrough to a singlegarminconnectmethod), but now return a structured{"status": "success" | "mfa_required" | "failed", "message": "..."}object instead of a string Go has to pattern-match.call— a fully generic dispatcher:getattr(garmin_client, params["method"])(**params["args"]), JSON-serializing whatever it returns. This is what makes everygarminconnectmethod reachable without new Python code per method — including future needs (sleep, HRV, stress, body battery) and, by the same generic mechanism, mutating/destructive methods (upload_activity,delete_activity, etc.). No allowlist is enforced in Python; correctness of what Go chooses to call is Go's responsibility, same as calling any other library method directly would be.
id lets Go assert a response matches its in-flight request — calls are already serialized by the existing sync.Mutex in mcpClient/its replacement, so this is a desync-detection assertion, not a concurrency mechanism.
Go's line reader uses a raised buffer size (well above the default 64KB bufio.Scanner limit) since get_activity_details responses (per-second telemetry for a full run) can be several MB.
Components
Python wrapper — backend/garmin-wrapper/wrapper.py
- Single file. Same shape as today's
server.pyminus all MCP scaffolding (FastMCP,@mcp.tool()decorators,mcp.run()). - Embedded into the Go binary at compile time via
//go:embed wrapper.py(a[]byte/stringconstant ininternal/garmin), rather than referenced by a configurable filesystem path. At process start,ensureStartedwrites the embedded source to a temp file (os.CreateTemp) once per process lifetime and execs the configured Python interpreter against that path. This removes any notion of a "server script location" from configuration entirely — whateverwrapper.pyshipped inside the running binary is exactly what gets executed, so there's no drift between binary version and script version, and the container image needs no separateCOPYstep for it. - Reads
GARMIN_EMAIL/GARMIN_PASSWORD/GARMIN_TOKENSTOREfrom the environment at process start — unchanged from today. - Holds one module-level
Garminclient instance and an auth-state variable (unauthenticated/mfa_pending/authenticated). - Main loop:
for line in sys.stdin, parse{"id", "cmd", "params"}, dispatch to theauthenticate/complete_mfa/callhandler, write exactly one JSON line to stdout per response, flush immediately. Debug/diagnostic output continues to go to stderr only (stdout is reserved for the response protocol) — Go already drains stderr today and keeps doing so unchanged. - Any exception raised by a
calldispatch (bad method name viaAttributeError,GarminConnectAuthenticationError, network errors, anything else) is caught at the top-level dispatch loop and turned into{"id": N, "error": "<message>"}— it must never let a raw Python traceback hit stdout, since that would corrupt the line-based framing. - Dependency:
garminconnectonly. Dropmcp[cli].
Go client — internal/garmin/client.go
- Same
mu sync.Mutex+ lazyensureStarted/startedpattern as today. The mcp-go stdio transport is replaced withos/exec.Cmdplus abufio.Scanner(raised buffer) overStdoutand ajson.EncoderoverStdin. UpdateCredentialsstill tears down (kill subprocess) and lets the next call respawn fresh, unchanged.Closestill terminates the subprocess.- Each
Clientinterface method builds the appropriate request (authenticate/complete_mfadirectly, or acallenvelope naming the rightgarminconnectmethod forGetActivities→get_activities_by_date,GetActivitySplits→get_activity_splits,GetActivityDetails→get_activity_details,GetWorkoutByID→get_workout_by_id), writes+flushes it, reads one response line, and unmarshalsresultinto the existing typed structs (Activity,ActivitySplits,ActivityDetails,Workout— unchanged; the wrapper still forwardsgarminconnect's own JSON-compatible dicts/lists as-is). AuthResult/AuthStatustypes are unchanged, butparseAuthResult's string-matching (client.go:189-198) is deleted entirely — the wrapper'sstatusfield maps directly toAuthSuccess/AuthMFARequired/AuthFailed.- The interface itself gains no new methods yet (YAGNI) — the generic
callcommand exists at the wire-protocol level so a future typed method (e.g. for sleep/HRV data, if the AI training-plan features end up needing it) is a small, self-contained addition when actually needed, not a protocol change.
Error handling
- Python: every
calldispatch exception is caught and returned as{"error": "<message>"}.authenticate/complete_mfanever raise past their own handlers (matches today). - Go: a non-nil
errorfield in a response becomesfmt.Errorf("call %s: %s", method, resp.Error)(or equivalent forauthenticate/complete_mfa), the same shapeinternal/syncalready receives and wraps further today (fmt.Errorf("get_activities(...): %w", err), etc.) — confirmed no code downstream branches on specific error text (e.g. no "Authentication error" string-matching ininternal/sync/internal/api), so no structured error-type field is needed beyond the plain message. - Process-level failures (subprocess exited, pipe closed, malformed JSON line) surface as a Go error from the read/write call.
startedis not auto-reset on such failures — matching today's behavior where a crashed subprocess just errors on next use untilUpdateCredentialsforces a respawn.
Process lifecycle & testing
- Lifecycle unchanged: lazy-start on first call, one persistent subprocess per user (kept warm across calls to preserve the Garmin session/tokenstore), torn down and respawned only by
UpdateCredentials. internal/garmin/mock.Clientis untouched — it fakes theClientinterface, not the transport, sointernal/sync/internal/apitests need no changes.- New/updated tests in
internal/garmin:- Framing round-trip: a request encodes correctly, a response line decodes correctly, and a large (multi-MB)
get_activity_details-shaped payload doesn't overflow the scanner buffer. - Structured auth status mapping (replacing today's
parseAuthResultstring-matching tests). - Error propagation: a
{"error": ...}response becomes a Go error with the expected message. - These can run against a stub script (not the real
wrapper.py/garminconnect) that speaks the protocol, mirroring howclient_test.goalready avoids hitting real Garmin.
- Framing round-trip: a request encodes correctly, a response line decodes correctly, and a large (multi-MB)
Migration & container
- Full replacement, no compatibility shim, consistent with this project's "no migration history" stance elsewhere:
mcp-goimport and all MCP-specific code inclient.goare deleted in the same change, not kept behind a flag. backend/start.sh'sMCP_GARMIN_PYTHON/MCP_GARMIN_SERVERenv vars are retired, not just renamed. Only one config knob remains:GARMIN_WRAPPER_PYTHON, and it becomes optional (default"python3", resolved viaPATH) rather than required — matching howconfig.Load()already treats genuinely-optional plumbing (e.g.GENIUSRUN_OIDC_REQUIRED_ROLE). There is no longer a script-location env var at all, sincewrapper.pyis embedded in the binary (see Components above). Local dev can still pointGARMIN_WRAPPER_PYTHONat a venv interpreter if desired; it's just no longer required to.- Container image only needs: a Python runtime with
garminconnectinstalled (pip install garminconnectin the image, no venv necessary since the container itself is the isolation boundary) — no second repo to clone/vendor, nomcpSDK on either side, and no separate wrapper script file toCOPYin. - The standalone
mcp-garminrepo is retired (archived) once this ships; its test suite (tests/test_server.py) either gets ported to testwrapper.pydirectly inside geniusrun, or is dropped in favor of the newinternal/garmintests above, whichever ends up covering the same ground with less duplication — a call to make during implementation, not part of this spec.
Rollout notes
- No data migration involved — this only changes how the Go backend talks to Garmin, not what's stored.
- Existing per-user token stores under
GarminTokenStoreRootare unaffected (sameGARMIN_TOKENSTOREenv var name passed to the subprocess, same directory layout) — no re-login required for existing users after cutover. - Deploy order: ship the new wrapper + Go client together as one change (interface unchanged, so this is an internal swap, not a rolling/compat-sensitive migration).