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.
1753 lines
63 KiB
Markdown
1753 lines
63 KiB
Markdown
# Drop MCP for Garmin integration — implementation plan
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** Replace `internal/garmin`'s MCP-based `mcp-garmin` integration with a small Python subprocess wrapper (embedded in this repo) that imports `garminconnect` directly and speaks a minimal newline-delimited-JSON protocol, so the `garmin.Client` interface's behavior is preserved with fewer dependencies and no external repo to vendor into a container.
|
||
|
||
**Architecture:** `internal/garmin/pyscript/wrapper.py` (embedded via `go:embed`) replaces `mcp-garmin`'s `server.py`; `internal/garmin/client.go`'s `subprocessClient` replaces `mcpClient`, driving the script over `os/exec` + JSON-lines instead of MCP/JSON-RPC. The `garmin.Client` interface, `internal/sync`, and `internal/api` are untouched.
|
||
|
||
**Tech Stack:** Go (`os/exec`, `bufio`, `encoding/json`, `embed`), Python 3.11+ (`garminconnect`, `pytest` for the wrapper's own tests).
|
||
|
||
## Global Constraints
|
||
|
||
- Full replacement, no compatibility shim — delete the MCP-based code in the same change, don't keep it behind a flag.
|
||
- `garmin.Client`'s interface (`Authenticate`, `CompleteMFA`, `UpdateCredentials`, `GetActivities`, `GetActivitySplits`, `GetActivityDetails`, `GetWorkoutByID`, `Close`) must not change signature — `internal/sync` and `internal/api` require zero edits.
|
||
- Wire protocol: one JSON object per line each direction — `{"id","cmd","params"}` requests, `{"id","result"}` / `{"id","error"}` responses. `cmd` is `authenticate`, `complete_mfa`, or generic `call` (`{"method","args"}` → `getattr(client, method)(**args)`), per `docs/superpowers/specs/2026-07-25-garmin-direct-wrapper-design.md`.
|
||
- `wrapper.py` is embedded into the Go binary via `//go:embed pyscript/wrapper.py` — no script-path env var of any kind.
|
||
- `GARMIN_WRAPPER_PYTHON` is optional, defaulting to `"python3"` (resolved via `PATH`) — `MCP_GARMIN_PYTHON`/`MCP_GARMIN_SERVER` are retired entirely, not renamed.
|
||
- `garminconnect`'s actual method kwargs must be used exactly: `get_activities_by_date(startdate, enddate)`, `get_activity_splits(activity_id)`, `get_activity_details(activity_id)`, `get_workout_by_id(workout_id)` (confirmed via `inspect.signature` against the installed package — note `startdate`/`enddate`, not `start_date`/`end_date`).
|
||
- `gofmt -l .` must report nothing; `go build ./...`, `go vet ./...`, and `go test ./...` must all pass before the final commit.
|
||
- `pytest` must pass in `backend/internal/garmin/pyscript/`.
|
||
|
||
---
|
||
|
||
### Task 1: Python wrapper (`wrapper.py`) with its own test suite
|
||
|
||
**Files:**
|
||
- Create: `backend/internal/garmin/pyscript/wrapper.py`
|
||
- Create: `backend/internal/garmin/pyscript/pyproject.toml`
|
||
- Create: `backend/internal/garmin/pyscript/.gitignore`
|
||
- Test: `backend/internal/garmin/pyscript/tests/__init__.py`
|
||
- Test: `backend/internal/garmin/pyscript/tests/test_wrapper.py`
|
||
|
||
**Interfaces:**
|
||
- Produces: `wrapper.dispatch(req: dict) -> dict` — the single entry point Go's subprocess talks to indirectly via stdin/stdout; also `wrapper._auth_state`, `wrapper._client`, `wrapper._mfa_input_queue`, `wrapper._login_result_queue`, `wrapper.Garmin` (re-exported name, patchable in tests), used only within this task's own test suite.
|
||
|
||
- [ ] **Step 1: Create the Python project manifest**
|
||
|
||
`backend/internal/garmin/pyscript/pyproject.toml`:
|
||
|
||
```toml
|
||
[project]
|
||
name = "geniusrun-garmin-wrapper"
|
||
version = "0.1.0"
|
||
description = "Subprocess wrapper around garminconnect for geniusrund's internal/garmin package"
|
||
requires-python = ">=3.11"
|
||
dependencies = [
|
||
"garminconnect",
|
||
]
|
||
|
||
[project.optional-dependencies]
|
||
dev = ["pytest"]
|
||
|
||
[build-system]
|
||
requires = ["hatchling"]
|
||
build-backend = "hatchling.build"
|
||
```
|
||
|
||
`backend/internal/garmin/pyscript/.gitignore`:
|
||
|
||
```
|
||
.venv/
|
||
__pycache__/
|
||
.pytest_cache/
|
||
*.pyc
|
||
```
|
||
|
||
- [ ] **Step 2: Create a local venv and install dependencies**
|
||
|
||
Run:
|
||
```bash
|
||
cd backend/internal/garmin/pyscript
|
||
python3 -m venv .venv
|
||
.venv/bin/pip install -e ".[dev]"
|
||
```
|
||
Expected: installs `garminconnect` and `pytest` into `.venv` with no errors.
|
||
|
||
- [ ] **Step 3: Write the failing test suite**
|
||
|
||
`backend/internal/garmin/pyscript/tests/__init__.py`: empty file.
|
||
|
||
`backend/internal/garmin/pyscript/tests/test_wrapper.py`:
|
||
|
||
```python
|
||
import os
|
||
import queue
|
||
from unittest.mock import MagicMock, patch
|
||
|
||
import pytest
|
||
|
||
import wrapper
|
||
|
||
|
||
@pytest.fixture(autouse=True)
|
||
def reset_state():
|
||
original_state = wrapper._auth_state
|
||
original_client = wrapper._client
|
||
for q in (wrapper._mfa_input_queue, wrapper._login_result_queue):
|
||
while not q.empty():
|
||
try:
|
||
q.get_nowait()
|
||
except queue.Empty:
|
||
break
|
||
yield
|
||
wrapper._auth_state = original_state
|
||
wrapper._client = original_client
|
||
|
||
|
||
def test_authenticate_success():
|
||
env = {"GARMIN_EMAIL": "test@example.com", "GARMIN_PASSWORD": "secret"}
|
||
with patch.dict("os.environ", env):
|
||
with patch("wrapper.Garmin") as mock_garmin_cls:
|
||
mock_garmin_cls.return_value = MagicMock()
|
||
resp = wrapper.dispatch({"id": 1, "cmd": "authenticate"})
|
||
assert resp == {"id": 1, "result": {"status": "success", "message": "Authenticated successfully."}}
|
||
assert wrapper._auth_state == "authenticated"
|
||
|
||
|
||
def test_authenticate_mfa_required():
|
||
env = {"GARMIN_EMAIL": "test@example.com", "GARMIN_PASSWORD": "secret"}
|
||
with patch.dict("os.environ", env):
|
||
with patch("wrapper.Garmin") as mock_garmin_cls:
|
||
mock_garmin_cls.return_value = MagicMock()
|
||
with patch.object(wrapper._login_result_queue, "get", side_effect=queue.Empty):
|
||
resp = wrapper.dispatch({"id": 2, "cmd": "authenticate"})
|
||
assert resp["result"]["status"] == "mfa_required"
|
||
assert wrapper._auth_state == "mfa_pending"
|
||
|
||
|
||
def test_authenticate_missing_credentials():
|
||
with patch.dict("os.environ", {}, clear=False):
|
||
os.environ.pop("GARMIN_EMAIL", None)
|
||
os.environ.pop("GARMIN_PASSWORD", None)
|
||
resp = wrapper.dispatch({"id": 3, "cmd": "authenticate"})
|
||
assert resp["result"]["status"] == "failed"
|
||
assert "GARMIN_EMAIL" in resp["result"]["message"]
|
||
|
||
|
||
def test_complete_mfa_success():
|
||
wrapper._auth_state = "mfa_pending"
|
||
wrapper._login_result_queue.put(("success", None))
|
||
resp = wrapper.dispatch({"id": 4, "cmd": "complete_mfa", "params": {"code": "123456"}})
|
||
assert resp == {
|
||
"id": 4,
|
||
"result": {"status": "success", "message": "MFA accepted. Authenticated successfully."},
|
||
}
|
||
assert wrapper._auth_state == "authenticated"
|
||
assert wrapper._mfa_input_queue.get_nowait() == "123456"
|
||
|
||
|
||
def test_complete_mfa_not_in_progress():
|
||
wrapper._auth_state = "unauthenticated"
|
||
resp = wrapper.dispatch({"id": 5, "cmd": "complete_mfa", "params": {"code": "123456"}})
|
||
assert resp["result"]["status"] == "failed"
|
||
assert "No MFA in progress" in resp["result"]["message"]
|
||
|
||
|
||
def test_call_dispatches_to_named_garminconnect_method():
|
||
wrapper._auth_state = "authenticated"
|
||
wrapper._client = MagicMock()
|
||
wrapper._client.get_activities_by_date.return_value = [{"activityId": "111"}]
|
||
|
||
resp = wrapper.dispatch({
|
||
"id": 6,
|
||
"cmd": "call",
|
||
"params": {
|
||
"method": "get_activities_by_date",
|
||
"args": {"startdate": "2026-07-01", "enddate": "2026-07-25"},
|
||
},
|
||
})
|
||
|
||
assert resp == {"id": 6, "result": [{"activityId": "111"}]}
|
||
wrapper._client.get_activities_by_date.assert_called_once_with(
|
||
startdate="2026-07-01", enddate="2026-07-25"
|
||
)
|
||
|
||
|
||
def test_call_unauthenticated_is_error():
|
||
wrapper._auth_state = "unauthenticated"
|
||
resp = wrapper.dispatch({
|
||
"id": 7, "cmd": "call", "params": {"method": "get_activities_by_date", "args": {}},
|
||
})
|
||
assert "error" in resp
|
||
assert "Not authenticated" in resp["error"]
|
||
|
||
|
||
def test_call_unknown_method_is_error():
|
||
wrapper._auth_state = "authenticated"
|
||
wrapper._client = MagicMock(spec=["get_activities_by_date"])
|
||
resp = wrapper.dispatch({
|
||
"id": 8, "cmd": "call", "params": {"method": "delete_everything", "args": {}},
|
||
})
|
||
assert resp["id"] == 8
|
||
assert "error" in resp
|
||
|
||
|
||
def test_call_propagates_garminconnect_exception_as_error():
|
||
wrapper._auth_state = "authenticated"
|
||
wrapper._client = MagicMock()
|
||
wrapper._client.get_activity_splits.side_effect = Exception("not found")
|
||
resp = wrapper.dispatch({
|
||
"id": 9, "cmd": "call", "params": {"method": "get_activity_splits", "args": {"activity_id": "999"}},
|
||
})
|
||
assert resp == {"id": 9, "error": "not found"}
|
||
|
||
|
||
def test_dispatch_unknown_cmd_is_error():
|
||
resp = wrapper.dispatch({"id": 10, "cmd": "not_a_real_cmd"})
|
||
assert resp["id"] == 10
|
||
assert "unknown cmd" in resp["error"]
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they fail**
|
||
|
||
Run: `cd backend/internal/garmin/pyscript && .venv/bin/pytest -v`
|
||
Expected: `ModuleNotFoundError: No module named 'wrapper'` (or collection failure) — `wrapper.py` doesn't exist yet.
|
||
|
||
- [ ] **Step 5: Write `wrapper.py`**
|
||
|
||
`backend/internal/garmin/pyscript/wrapper.py`:
|
||
|
||
```python
|
||
"""Subprocess wrapper around garminconnect, spoken to over newline-delimited
|
||
JSON on stdin/stdout by geniusrund's internal/garmin package. See
|
||
docs/superpowers/specs/2026-07-25-garmin-direct-wrapper-design.md."""
|
||
|
||
import json
|
||
import os
|
||
import queue
|
||
import sys
|
||
import threading
|
||
import traceback
|
||
|
||
from garminconnect import Garmin
|
||
|
||
TOKENSTORE = os.path.expanduser(os.environ.get("GARMIN_TOKENSTORE", "~/.garth"))
|
||
|
||
_client = None
|
||
_auth_state = "unauthenticated"
|
||
_mfa_input_queue = queue.Queue()
|
||
_login_result_queue = queue.Queue()
|
||
|
||
|
||
def _debug(msg):
|
||
print(f"[garmin-wrapper debug] {msg}", file=sys.stderr, flush=True)
|
||
|
||
|
||
def _prompt_mfa():
|
||
_debug("garminconnect invoked prompt_mfa() -- this is a REAL MFA challenge from Garmin")
|
||
code = _mfa_input_queue.get(timeout=300)
|
||
_debug(f"prompt_mfa() handing code of length {len(code)} back to garminconnect")
|
||
return code
|
||
|
||
|
||
def _startup_login():
|
||
"""Silently resume a cached tokenstore session at process start, so a
|
||
freshly (re)spawned subprocess is already authenticated for background
|
||
syncs that never call the explicit authenticate command."""
|
||
global _client, _auth_state
|
||
email = os.environ.get("GARMIN_EMAIL")
|
||
password = os.environ.get("GARMIN_PASSWORD")
|
||
if not email or not password:
|
||
return
|
||
try:
|
||
_client = Garmin(email, password)
|
||
_client.login(tokenstore=TOKENSTORE)
|
||
_auth_state = "authenticated"
|
||
except Exception as exc:
|
||
_debug(f"startup tokenstore login failed: {exc}")
|
||
_auth_state = "unauthenticated"
|
||
|
||
|
||
def _handle_authenticate(_params):
|
||
global _client, _auth_state
|
||
|
||
email = os.environ.get("GARMIN_EMAIL", "")
|
||
password = os.environ.get("GARMIN_PASSWORD", "")
|
||
if not email or not password:
|
||
return {
|
||
"status": "failed",
|
||
"message": "GARMIN_EMAIL and GARMIN_PASSWORD environment variables are required.",
|
||
}
|
||
|
||
def _do_login():
|
||
_debug(f"background login thread starting _client.login(tokenstore={TOKENSTORE})")
|
||
try:
|
||
_client.login(tokenstore=TOKENSTORE)
|
||
_debug("_client.login() returned successfully")
|
||
_login_result_queue.put(("success", None))
|
||
except Exception as exc:
|
||
_debug(f"_client.login() raised {type(exc).__name__}: {exc}")
|
||
_debug(traceback.format_exc())
|
||
_login_result_queue.put(("error", str(exc)))
|
||
|
||
_client = Garmin(email, password)
|
||
_client.prompt_mfa = _prompt_mfa
|
||
threading.Thread(target=_do_login, daemon=True).start()
|
||
|
||
try:
|
||
status, err = _login_result_queue.get(timeout=10)
|
||
_debug(f"authenticate got result within 10s timeout: status={status}")
|
||
if status == "success":
|
||
_auth_state = "authenticated"
|
||
return {"status": "success", "message": "Authenticated successfully."}
|
||
return {"status": "failed", "message": f"Authentication failed: {err}"}
|
||
except queue.Empty:
|
||
_debug(
|
||
"authenticate hit the 10s timeout with no result yet -- reporting mfa_required, "
|
||
"but this does NOT necessarily mean prompt_mfa() was actually invoked; check "
|
||
"whether the 'REAL MFA challenge' debug line above appears to tell real MFA "
|
||
"apart from a merely slow login."
|
||
)
|
||
_auth_state = "mfa_pending"
|
||
return {
|
||
"status": "mfa_required",
|
||
"message": "MFA required. Garmin has sent a verification code to your registered email or phone.",
|
||
}
|
||
|
||
|
||
def _handle_complete_mfa(params):
|
||
global _auth_state
|
||
|
||
if _auth_state != "mfa_pending":
|
||
return {"status": "failed", "message": "No MFA in progress. Call authenticate first."}
|
||
|
||
code = params["code"]
|
||
_debug(f"complete_mfa received a code of length {len(code)}, pushing to mfa queue")
|
||
_mfa_input_queue.put(code)
|
||
|
||
try:
|
||
status, err = _login_result_queue.get(timeout=30)
|
||
_debug(f"complete_mfa got result: status={status} err={err}")
|
||
if status == "success":
|
||
_auth_state = "authenticated"
|
||
return {"status": "success", "message": "MFA accepted. Authenticated successfully."}
|
||
return {"status": "failed", "message": f"Authentication failed after MFA: {err}"}
|
||
except queue.Empty:
|
||
_auth_state = "unauthenticated"
|
||
return {
|
||
"status": "failed",
|
||
"message": "Timed out waiting for authentication to complete. Call authenticate again.",
|
||
}
|
||
|
||
|
||
def _handle_call(params):
|
||
if _auth_state != "authenticated":
|
||
raise RuntimeError("Not authenticated. Call authenticate first.")
|
||
method = params["method"]
|
||
args = params.get("args") or {}
|
||
fn = getattr(_client, method)
|
||
return fn(**args)
|
||
|
||
|
||
_HANDLERS = {
|
||
"authenticate": _handle_authenticate,
|
||
"complete_mfa": _handle_complete_mfa,
|
||
"call": _handle_call,
|
||
}
|
||
|
||
|
||
def dispatch(req):
|
||
handler = _HANDLERS.get(req.get("cmd"))
|
||
if handler is None:
|
||
return {"id": req.get("id"), "error": f"unknown cmd {req.get('cmd')!r}"}
|
||
try:
|
||
result = handler(req.get("params") or {})
|
||
return {"id": req["id"], "result": result}
|
||
except Exception as exc:
|
||
_debug(f"{req.get('cmd')} raised {type(exc).__name__}: {exc}")
|
||
_debug(traceback.format_exc())
|
||
return {"id": req.get("id"), "error": str(exc)}
|
||
|
||
|
||
def main():
|
||
_startup_login()
|
||
for line in sys.stdin:
|
||
line = line.strip()
|
||
if not line:
|
||
continue
|
||
req = json.loads(line)
|
||
resp = dispatch(req)
|
||
print(json.dumps(resp), flush=True)
|
||
|
||
|
||
if __name__ == "__main__":
|
||
main()
|
||
```
|
||
|
||
- [ ] **Step 6: Run the tests to verify they pass**
|
||
|
||
Run: `cd backend/internal/garmin/pyscript && .venv/bin/pytest -v`
|
||
Expected: all tests PASS.
|
||
|
||
- [ ] **Step 7: Commit**
|
||
|
||
```bash
|
||
git add backend/internal/garmin/pyscript
|
||
git commit -m "$(cat <<'EOF'
|
||
feat(garmin): add direct garminconnect wrapper script
|
||
|
||
Replaces mcp-garmin's server.py: a JSON-lines subprocess protocol
|
||
(authenticate/complete_mfa/call) around garminconnect directly, no MCP.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 2: Go transport scaffolding (`subprocessClient`, wire protocol, lifecycle)
|
||
|
||
**Files:**
|
||
- Modify: `backend/internal/garmin/client.go` (full rewrite)
|
||
- Modify: `backend/internal/garmin/client_test.go` (full rewrite)
|
||
|
||
**Interfaces:**
|
||
- Consumes: nothing from other tasks.
|
||
- Produces: `Config{PythonPath, GarminEmail, GarminPassword, TokenStorePath string}`, `NewClient(cfg Config) Client`, `subprocessClient` (unexported struct implementing `Client`), `wireRequest{ID int; Cmd string; Params any}`, `wireResponse{ID int; Result json.RawMessage; Error string}`, `callParams{Method string; Args map[string]any}`, `(*subprocessClient).roundTrip(cmd string, params any) (json.RawMessage, error)`, `(*subprocessClient).ensureStarted() error`, `(*subprocessClient).close() error` (unexported, no-lock; `Close()`/`UpdateCredentials` call it while already holding `c.mu`). Later tasks (3, 4) add methods to `subprocessClient` using `roundTrip`/`ensureStarted` and must not redefine these types.
|
||
|
||
- [ ] **Step 1: Write the failing tests for the transport layer**
|
||
|
||
`backend/internal/garmin/client_test.go` (replaces the old file entirely — `parseAuthResult`/`mcpClient` no longer exist):
|
||
|
||
```go
|
||
package garmin
|
||
|
||
import (
|
||
"bufio"
|
||
"encoding/json"
|
||
"io"
|
||
"strings"
|
||
"testing"
|
||
)
|
||
|
||
// wireResponsePayload is what a fake wrapper handler returns for one
|
||
// request; the harness fills in the response ID.
|
||
type wireResponsePayload struct {
|
||
result json.RawMessage
|
||
err string
|
||
}
|
||
|
||
func fakeResult(v any) wireResponsePayload {
|
||
b, err := json.Marshal(v)
|
||
if err != nil {
|
||
panic(err)
|
||
}
|
||
return wireResponsePayload{result: b}
|
||
}
|
||
|
||
func fakeError(msg string) wireResponsePayload {
|
||
return wireResponsePayload{err: msg}
|
||
}
|
||
|
||
// newFakeWrapperClient wires a subprocessClient to an in-process goroutine
|
||
// that plays the Python wrapper's role, so protocol-level Go logic can be
|
||
// tested without python3/garminconnect installed.
|
||
func newFakeWrapperClient(t *testing.T, handle func(cmd string, params json.RawMessage) wireResponsePayload) *subprocessClient {
|
||
t.Helper()
|
||
|
||
reqR, reqW := io.Pipe()
|
||
respR, respW := io.Pipe()
|
||
t.Cleanup(func() {
|
||
reqW.Close()
|
||
respW.Close()
|
||
})
|
||
|
||
go func() {
|
||
scanner := bufio.NewScanner(reqR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
enc := json.NewEncoder(respW)
|
||
for scanner.Scan() {
|
||
var req struct {
|
||
ID int `json:"id"`
|
||
Cmd string `json:"cmd"`
|
||
Params json.RawMessage `json:"params"`
|
||
}
|
||
if err := json.Unmarshal(scanner.Bytes(), &req); err != nil {
|
||
continue
|
||
}
|
||
payload := handle(req.Cmd, req.Params)
|
||
resp := wireResponse{ID: req.ID, Result: payload.result, Error: payload.err}
|
||
if err := enc.Encode(resp); err != nil {
|
||
return
|
||
}
|
||
}
|
||
}()
|
||
|
||
scanner := bufio.NewScanner(respR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
|
||
return &subprocessClient{
|
||
started: true,
|
||
enc: json.NewEncoder(reqW),
|
||
scanner: scanner,
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_RoundTrip_DetectsIDMismatch(t *testing.T) {
|
||
reqR, reqW := io.Pipe()
|
||
respR, respW := io.Pipe()
|
||
defer reqW.Close()
|
||
defer respW.Close()
|
||
|
||
go func() {
|
||
scanner := bufio.NewScanner(reqR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
scanner.Scan() // read and discard the one request
|
||
json.NewEncoder(respW).Encode(wireResponse{ID: 999, Result: json.RawMessage(`{}`)})
|
||
}()
|
||
|
||
scanner := bufio.NewScanner(respR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
c := &subprocessClient{started: true, enc: json.NewEncoder(reqW), scanner: scanner}
|
||
|
||
_, err := c.roundTrip("authenticate", nil)
|
||
if err == nil || !strings.Contains(err.Error(), "id mismatch") {
|
||
t.Fatalf("roundTrip error = %v, want an id mismatch error", err)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_RoundTrip_SubprocessClosedIsError(t *testing.T) {
|
||
reqR, reqW := io.Pipe()
|
||
respR, respW := io.Pipe()
|
||
defer reqW.Close()
|
||
|
||
go func() {
|
||
scanner := bufio.NewScanner(reqR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
scanner.Scan()
|
||
respW.Close() // subprocess "exited": stdout closes with no response
|
||
}()
|
||
|
||
scanner := bufio.NewScanner(respR)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
c := &subprocessClient{started: true, enc: json.NewEncoder(reqW), scanner: scanner}
|
||
|
||
_, err := c.roundTrip("authenticate", nil)
|
||
if err == nil || !strings.Contains(err.Error(), "closed") {
|
||
t.Fatalf("roundTrip error = %v, want a subprocess-closed error", err)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_RoundTrip_WrapperErrorPropagates(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
return fakeError("boom")
|
||
})
|
||
|
||
_, err := c.roundTrip("authenticate", nil)
|
||
if err == nil || !strings.Contains(err.Error(), "boom") {
|
||
t.Fatalf("roundTrip error = %v, want it to mention %q", err, "boom")
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_UpdateCredentials_ResetsStartedState(t *testing.T) {
|
||
c := &subprocessClient{cfg: Config{GarminEmail: "old@example.com", GarminPassword: "old"}}
|
||
c.started = true // simulate an already-spawned subprocess, no real cmd/pipes
|
||
|
||
c.UpdateCredentials("new@example.com", "new")
|
||
|
||
if c.cfg.GarminEmail != "new@example.com" || c.cfg.GarminPassword != "new" {
|
||
t.Errorf("cfg after update = %+v, want new@example.com/new", c.cfg)
|
||
}
|
||
if c.started {
|
||
t.Error("started should be reset to false so the next call respawns the subprocess")
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_Close_NoOpWhenNotStarted(t *testing.T) {
|
||
c := &subprocessClient{}
|
||
if err := c.Close(); err != nil {
|
||
t.Errorf("Close on an unstarted client = %v, want nil", err)
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run the tests to verify they fail**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -run TestSubprocessClient -v`
|
||
Expected: compile failure — `subprocessClient`, `wireResponse`, `roundTrip`, `maxWrapperLineBytes`, `Config` (new shape) don't exist yet.
|
||
|
||
- [ ] **Step 3: Rewrite `client.go`**
|
||
|
||
`backend/internal/garmin/client.go`:
|
||
|
||
```go
|
||
// Package garmin wraps a direct garminconnect subprocess (see
|
||
// pyscript/wrapper.py) as a narrow Go client interface, so the rest of
|
||
// geniusrun never deals with the wire protocol directly.
|
||
package garmin
|
||
|
||
import (
|
||
"bufio"
|
||
"context"
|
||
_ "embed"
|
||
"encoding/json"
|
||
"fmt"
|
||
"io"
|
||
"os"
|
||
"os/exec"
|
||
"sync"
|
||
)
|
||
|
||
//go:embed pyscript/wrapper.py
|
||
var wrapperScript string
|
||
|
||
// maxWrapperLineBytes bounds one JSON-line response from the wrapper
|
||
// subprocess -- well above the default 64KB bufio.Scanner limit, since
|
||
// get_activity_details responses (per-second telemetry) can be several MB.
|
||
const maxWrapperLineBytes = 16 * 1024 * 1024
|
||
|
||
// Client is the interface the rest of geniusrun depends on. The real
|
||
// implementation drives an embedded Python wrapper subprocess over stdio;
|
||
// internal/garmin/mock provides a fake for tests and frontend-only
|
||
// development.
|
||
type Client interface {
|
||
// Authenticate triggers Garmin login using credentials the subprocess
|
||
// was started with. Spawns the subprocess on first call.
|
||
Authenticate(ctx context.Context) (AuthResult, error)
|
||
// CompleteMFA submits an MFA code for a login started by Authenticate.
|
||
CompleteMFA(ctx context.Context, code string) (AuthResult, error)
|
||
// UpdateCredentials replaces the Garmin email/password used to spawn
|
||
// the subprocess, and terminates any already-running subprocess (which
|
||
// would otherwise still be authenticated under the old credentials).
|
||
// The next call that needs the subprocess spawns a fresh one with the
|
||
// new credentials.
|
||
UpdateCredentials(email, password string)
|
||
// GetActivities lists activities between start and end (YYYY-MM-DD).
|
||
GetActivities(ctx context.Context, startDate, endDate string, limit int) ([]Activity, error)
|
||
// GetActivitySplits fetches lap/split summaries for one activity.
|
||
GetActivitySplits(ctx context.Context, activityID int64) (ActivitySplits, error)
|
||
// GetActivityDetails fetches raw per-second telemetry for one activity.
|
||
GetActivityDetails(ctx context.Context, activityID int64) (ActivityDetails, error)
|
||
// GetWorkoutByID fetches a structured workout's step-by-step plan.
|
||
GetWorkoutByID(ctx context.Context, workoutID int64) (Workout, error)
|
||
// Close terminates the subprocess, if running.
|
||
Close() error
|
||
}
|
||
|
||
// Config configures how the Garmin wrapper subprocess is spawned.
|
||
type Config struct {
|
||
// PythonPath is the python3 interpreter to run the embedded wrapper
|
||
// script with. Empty defaults to "python3" resolved via PATH.
|
||
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 string
|
||
}
|
||
|
||
// wireRequest is one line sent to the wrapper subprocess's stdin.
|
||
type wireRequest struct {
|
||
ID int `json:"id"`
|
||
Cmd string `json:"cmd"`
|
||
Params any `json:"params,omitempty"`
|
||
}
|
||
|
||
// wireResponse is one line read from the wrapper subprocess's stdout.
|
||
type wireResponse struct {
|
||
ID int `json:"id"`
|
||
Result json.RawMessage `json:"result,omitempty"`
|
||
Error string `json:"error,omitempty"`
|
||
}
|
||
|
||
// callParams is the Params payload for a generic "call" request: dispatches
|
||
// to any garminconnect.Garmin method by name.
|
||
type callParams struct {
|
||
Method string `json:"method"`
|
||
Args map[string]any `json:"args,omitempty"`
|
||
}
|
||
|
||
// authResultWire is the Result payload for authenticate/complete_mfa.
|
||
type authResultWire struct {
|
||
Status string `json:"status"`
|
||
Message string `json:"message"`
|
||
}
|
||
|
||
func mapAuthStatus(s string) AuthStatus {
|
||
switch s {
|
||
case "success":
|
||
return AuthSuccess
|
||
case "mfa_required":
|
||
return AuthMFARequired
|
||
case "failed":
|
||
return AuthFailed
|
||
default:
|
||
return AuthUnknown
|
||
}
|
||
}
|
||
|
||
// subprocessClient is the real Client implementation, backed by a wrapper
|
||
// subprocess spoken to over newline-delimited JSON on stdio.
|
||
type subprocessClient struct {
|
||
cfg Config
|
||
|
||
mu sync.Mutex // serializes calls; the wrapper holds a single shared Garmin client
|
||
cmd *exec.Cmd
|
||
stdin io.WriteCloser
|
||
enc *json.Encoder
|
||
scanner *bufio.Scanner
|
||
scriptPath string // temp file holding the embedded wrapper.py, written once
|
||
started bool
|
||
nextID int
|
||
}
|
||
|
||
// ensureStarted spawns the wrapper subprocess if it isn't already running.
|
||
// Callers must hold c.mu.
|
||
func (c *subprocessClient) ensureStarted() error {
|
||
if c.started {
|
||
return nil
|
||
}
|
||
|
||
if c.scriptPath == "" {
|
||
f, err := os.CreateTemp("", "geniusrun-garmin-wrapper-*.py")
|
||
if err != nil {
|
||
return fmt.Errorf("write embedded wrapper script: %w", err)
|
||
}
|
||
if _, err := f.WriteString(wrapperScript); err != nil {
|
||
f.Close()
|
||
return fmt.Errorf("write embedded wrapper script: %w", err)
|
||
}
|
||
if err := f.Close(); err != nil {
|
||
return fmt.Errorf("write embedded wrapper script: %w", err)
|
||
}
|
||
c.scriptPath = f.Name()
|
||
}
|
||
|
||
pythonPath := c.cfg.PythonPath
|
||
if pythonPath == "" {
|
||
pythonPath = "python3"
|
||
}
|
||
|
||
cmd := exec.Command(pythonPath, c.scriptPath)
|
||
extraEnv := []string{
|
||
"GARMIN_EMAIL=" + c.cfg.GarminEmail,
|
||
"GARMIN_PASSWORD=" + c.cfg.GarminPassword,
|
||
"PYTHONUNBUFFERED=1",
|
||
}
|
||
if c.cfg.TokenStorePath != "" {
|
||
extraEnv = append(extraEnv, "GARMIN_TOKENSTORE="+c.cfg.TokenStorePath)
|
||
}
|
||
cmd.Env = append(os.Environ(), extraEnv...)
|
||
|
||
stdin, err := cmd.StdinPipe()
|
||
if err != nil {
|
||
return fmt.Errorf("open wrapper stdin: %w", err)
|
||
}
|
||
stdout, err := cmd.StdoutPipe()
|
||
if err != nil {
|
||
return fmt.Errorf("open wrapper stdout: %w", err)
|
||
}
|
||
stderr, err := cmd.StderrPipe()
|
||
if err != nil {
|
||
return fmt.Errorf("open wrapper stderr: %w", err)
|
||
}
|
||
|
||
if err := cmd.Start(); err != nil {
|
||
return fmt.Errorf("spawn garmin wrapper subprocess: %w", err)
|
||
}
|
||
go io.Copy(os.Stderr, stderr) // wrapper.py logs auth/rate-limit diagnostics to stderr
|
||
|
||
c.cmd = cmd
|
||
c.stdin = stdin
|
||
c.enc = json.NewEncoder(stdin)
|
||
scanner := bufio.NewScanner(stdout)
|
||
scanner.Buffer(make([]byte, 0, 64*1024), maxWrapperLineBytes)
|
||
c.scanner = scanner
|
||
c.started = true
|
||
c.nextID = 0
|
||
return nil
|
||
}
|
||
|
||
// roundTrip sends one request and returns its result payload, or an error
|
||
// if the wrapper reported one. Callers must hold c.mu and have already
|
||
// called ensureStarted.
|
||
func (c *subprocessClient) roundTrip(cmdName string, params any) (json.RawMessage, error) {
|
||
c.nextID++
|
||
id := c.nextID
|
||
|
||
if err := c.enc.Encode(wireRequest{ID: id, Cmd: cmdName, Params: params}); err != nil {
|
||
return nil, fmt.Errorf("write %s request: %w", cmdName, err)
|
||
}
|
||
|
||
if !c.scanner.Scan() {
|
||
if err := c.scanner.Err(); err != nil {
|
||
return nil, fmt.Errorf("read %s response: %w", cmdName, err)
|
||
}
|
||
return nil, fmt.Errorf("read %s response: subprocess closed its output", cmdName)
|
||
}
|
||
|
||
var resp wireResponse
|
||
if err := json.Unmarshal(c.scanner.Bytes(), &resp); err != nil {
|
||
return nil, fmt.Errorf("parse %s response: %w", cmdName, err)
|
||
}
|
||
if resp.ID != id {
|
||
return nil, fmt.Errorf("%s response id mismatch: got %d, want %d", cmdName, resp.ID, id)
|
||
}
|
||
if resp.Error != "" {
|
||
return nil, fmt.Errorf("%s: %s", cmdName, resp.Error)
|
||
}
|
||
return resp.Result, nil
|
||
}
|
||
|
||
// UpdateCredentials implements Client.
|
||
func (c *subprocessClient) UpdateCredentials(email, password string) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
c.cfg.GarminEmail = email
|
||
c.cfg.GarminPassword = password
|
||
c.close()
|
||
}
|
||
|
||
// Close implements Client.
|
||
func (c *subprocessClient) Close() error {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
return c.close()
|
||
}
|
||
|
||
// close terminates the subprocess, if running. Callers must hold c.mu.
|
||
func (c *subprocessClient) close() error {
|
||
if !c.started {
|
||
return nil
|
||
}
|
||
c.started = false
|
||
|
||
if c.stdin != nil {
|
||
c.stdin.Close()
|
||
}
|
||
if c.cmd != nil && c.cmd.Process != nil {
|
||
c.cmd.Process.Kill()
|
||
}
|
||
var err error
|
||
if c.cmd != nil {
|
||
err = c.cmd.Wait()
|
||
}
|
||
c.cmd, c.stdin, c.enc, c.scanner = nil, nil, nil, nil
|
||
return err
|
||
}
|
||
|
||
func truncate(s string, n int) string {
|
||
if len(s) <= n {
|
||
return s
|
||
}
|
||
return s[:n] + "...(truncated)"
|
||
}
|
||
```
|
||
|
||
Note: `Authenticate`, `CompleteMFA`, `GetActivities`, `GetActivitySplits`, `GetActivityDetails`, `GetWorkoutByID`, and `NewClient` are added in Tasks 3 and 4, not here. `NewClient` is deliberately deferred to Task 4: its signature returns the `Client` interface (`func NewClient(cfg Config) Client { return &subprocessClient{cfg: cfg} }`), and Go checks interface satisfaction at that return statement — since `*subprocessClient` doesn't implement `Authenticate`/`CompleteMFA`/the four `Get*` methods until Tasks 3–4 land, defining `NewClient` here would fail to compile. This task's own tests construct `&subprocessClient{...}` directly (never through `NewClient` or the `Client` interface), so they compile and pass without it.
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -run TestSubprocessClient -v`
|
||
Expected: all tests PASS. (A whole-repo `go build ./...` will still fail until Task 6 — `cmd/geniusrund`/`internal/api` still reference the old `garmin.Config.ServerPath` field and `garmin.NewClient`, neither of which exist yet. That's expected and fine mid-plan; don't run a full-repo build check until Task 6.)
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/internal/garmin/client.go backend/internal/garmin/client_test.go
|
||
git commit -m "$(cat <<'EOF'
|
||
feat(garmin): replace mcp-go transport with a JSON-lines subprocess client
|
||
|
||
subprocessClient spawns the embedded pyscript/wrapper.py over os/exec and
|
||
speaks newline-delimited JSON instead of MCP. Auth/data methods land in
|
||
follow-up commits; this is the transport + lifecycle plumbing only.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 3: Authenticate / CompleteMFA with structured status
|
||
|
||
**Files:**
|
||
- Modify: `backend/internal/garmin/client.go`
|
||
- Modify: `backend/internal/garmin/client_test.go`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `subprocessClient`, `roundTrip`, `ensureStarted`, `mapAuthStatus`, `authResultWire`, `newFakeWrapperClient`/`fakeResult`/`fakeError` from Task 2.
|
||
- Produces: `(*subprocessClient).Authenticate(ctx) (AuthResult, error)`, `(*subprocessClient).CompleteMFA(ctx, code) (AuthResult, error)`.
|
||
|
||
- [ ] **Step 1: Write the failing tests**
|
||
|
||
First, add `"context"` to `backend/internal/garmin/client_test.go`'s import block (the tests below are the first in this file to need it):
|
||
|
||
```go
|
||
import (
|
||
"bufio"
|
||
"context"
|
||
"encoding/json"
|
||
"io"
|
||
"strings"
|
||
"testing"
|
||
)
|
||
```
|
||
|
||
Then append to `backend/internal/garmin/client_test.go`:
|
||
|
||
```go
|
||
func TestSubprocessClient_Authenticate_Success(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
if cmd != "authenticate" {
|
||
t.Fatalf("unexpected cmd %q", cmd)
|
||
}
|
||
return fakeResult(authResultWire{Status: "success", Message: "Authenticated successfully."})
|
||
})
|
||
|
||
res, err := c.Authenticate(context.Background())
|
||
if err != nil {
|
||
t.Fatalf("Authenticate: %v", err)
|
||
}
|
||
if res.Status != AuthSuccess || res.Message != "Authenticated successfully." {
|
||
t.Errorf("Authenticate result = %+v", res)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_Authenticate_MFARequired(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
return fakeResult(authResultWire{Status: "mfa_required", Message: "MFA required. ..."})
|
||
})
|
||
|
||
res, err := c.Authenticate(context.Background())
|
||
if err != nil {
|
||
t.Fatalf("Authenticate: %v", err)
|
||
}
|
||
if res.Status != AuthMFARequired {
|
||
t.Errorf("Authenticate status = %v, want AuthMFARequired", res.Status)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_Authenticate_WrapperErrorPropagates(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
return fakeError("subprocess exploded")
|
||
})
|
||
|
||
_, err := c.Authenticate(context.Background())
|
||
if err == nil || !strings.Contains(err.Error(), "subprocess exploded") {
|
||
t.Fatalf("Authenticate error = %v, want it to mention the wrapper error", err)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_CompleteMFA_SendsCodeAndReturnsStatus(t *testing.T) {
|
||
var gotCode string
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
if cmd != "complete_mfa" {
|
||
t.Fatalf("unexpected cmd %q", cmd)
|
||
}
|
||
var p struct {
|
||
Code string `json:"code"`
|
||
}
|
||
if err := json.Unmarshal(params, &p); err != nil {
|
||
t.Fatalf("unmarshal params: %v", err)
|
||
}
|
||
gotCode = p.Code
|
||
return fakeResult(authResultWire{Status: "success", Message: "MFA accepted. Authenticated successfully."})
|
||
})
|
||
|
||
res, err := c.CompleteMFA(context.Background(), "123456")
|
||
if err != nil {
|
||
t.Fatalf("CompleteMFA: %v", err)
|
||
}
|
||
if gotCode != "123456" {
|
||
t.Errorf("code sent to wrapper = %q, want 123456", gotCode)
|
||
}
|
||
if res.Status != AuthSuccess {
|
||
t.Errorf("CompleteMFA status = %v, want AuthSuccess", res.Status)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_CompleteMFA_Failed(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
return fakeResult(authResultWire{Status: "failed", Message: "Authentication failed after MFA: bad code"})
|
||
})
|
||
|
||
res, err := c.CompleteMFA(context.Background(), "000000")
|
||
if err != nil {
|
||
t.Fatalf("CompleteMFA: %v", err)
|
||
}
|
||
if res.Status != AuthFailed {
|
||
t.Errorf("CompleteMFA status = %v, want AuthFailed", res.Status)
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run the tests to verify they fail**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -run TestSubprocessClient_Authenticate -v && go test ./internal/garmin/... -run TestSubprocessClient_CompleteMFA -v`
|
||
Expected: compile failure — `Authenticate`/`CompleteMFA` methods don't exist on `subprocessClient` yet.
|
||
|
||
- [ ] **Step 3: Implement `Authenticate`/`CompleteMFA`**
|
||
|
||
Append to `backend/internal/garmin/client.go` (after `roundTrip`):
|
||
|
||
```go
|
||
func (c *subprocessClient) Authenticate(ctx context.Context) (AuthResult, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return AuthResult{}, err
|
||
}
|
||
raw, err := c.roundTrip("authenticate", nil)
|
||
if err != nil {
|
||
return AuthResult{}, err
|
||
}
|
||
var wire authResultWire
|
||
if err := json.Unmarshal(raw, &wire); err != nil {
|
||
return AuthResult{}, fmt.Errorf("parse authenticate result: %w", err)
|
||
}
|
||
return AuthResult{Status: mapAuthStatus(wire.Status), Message: wire.Message}, nil
|
||
}
|
||
|
||
func (c *subprocessClient) CompleteMFA(ctx context.Context, code string) (AuthResult, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return AuthResult{}, err
|
||
}
|
||
raw, err := c.roundTrip("complete_mfa", map[string]any{"code": code})
|
||
if err != nil {
|
||
return AuthResult{}, err
|
||
}
|
||
var wire authResultWire
|
||
if err := json.Unmarshal(raw, &wire); err != nil {
|
||
return AuthResult{}, fmt.Errorf("parse complete_mfa result: %w", err)
|
||
}
|
||
return AuthResult{Status: mapAuthStatus(wire.Status), Message: wire.Message}, nil
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -run 'TestSubprocessClient_(Authenticate|CompleteMFA)' -v`
|
||
Expected: all PASS.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/internal/garmin/client.go backend/internal/garmin/client_test.go
|
||
git commit -m "$(cat <<'EOF'
|
||
feat(garmin): implement Authenticate/CompleteMFA on subprocessClient
|
||
|
||
Structured {status, message} responses replace the old string
|
||
pattern-matching (parseAuthResult) -- both sides of the protocol are now
|
||
owned by this repo, so there's no need to guess at phrasing anymore.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 4: Data-fetch methods via generic `call` dispatch
|
||
|
||
**Files:**
|
||
- Modify: `backend/internal/garmin/client.go`
|
||
- Modify: `backend/internal/garmin/client_test.go`
|
||
|
||
**Interfaces:**
|
||
- Consumes: everything from Tasks 2–3, plus existing `Activity`, `ActivitySplits`, `Lap`, `ActivityDetails`, `Workout` types from `types.go` (unchanged).
|
||
- Produces: `(*subprocessClient).GetActivities`, `GetActivitySplits`, `GetActivityDetails`, `GetWorkoutByID` — completing the `Client` interface.
|
||
|
||
- [ ] **Step 1: Write the failing tests**
|
||
|
||
Append to `backend/internal/garmin/client_test.go`:
|
||
|
||
```go
|
||
func TestSubprocessClient_GetActivities_UsesGarminconnectKwargNames(t *testing.T) {
|
||
var gotMethod string
|
||
var gotArgs map[string]any
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
var p callParams
|
||
if err := json.Unmarshal(params, &p); err != nil {
|
||
t.Fatalf("unmarshal params: %v", err)
|
||
}
|
||
gotMethod, gotArgs = p.Method, p.Args
|
||
return fakeResult([]map[string]any{
|
||
{"activityId": float64(123), "activityType": map[string]any{"typeKey": "running"}},
|
||
})
|
||
})
|
||
|
||
activities, err := c.GetActivities(context.Background(), "2026-07-01", "2026-07-25", 0)
|
||
if err != nil {
|
||
t.Fatalf("GetActivities: %v", err)
|
||
}
|
||
if gotMethod != "get_activities_by_date" {
|
||
t.Errorf("method = %q, want get_activities_by_date", gotMethod)
|
||
}
|
||
if gotArgs["startdate"] != "2026-07-01" || gotArgs["enddate"] != "2026-07-25" {
|
||
t.Errorf("args = %+v, want startdate/enddate matching garminconnect's real kwargs", gotArgs)
|
||
}
|
||
if len(activities) != 1 || activities[0].ActivityID != 123 {
|
||
t.Fatalf("activities = %+v", activities)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_GetActivities_AppliesLimitClientSide(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
raw := make([]map[string]any, 5)
|
||
for i := range raw {
|
||
raw[i] = map[string]any{"activityId": float64(i)}
|
||
}
|
||
return fakeResult(raw)
|
||
})
|
||
|
||
activities, err := c.GetActivities(context.Background(), "2026-07-01", "2026-07-25", 2)
|
||
if err != nil {
|
||
t.Fatalf("GetActivities: %v", err)
|
||
}
|
||
if len(activities) != 2 {
|
||
t.Fatalf("len(activities) = %d, want 2", len(activities))
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_GetActivitySplits_ParsesLapDTOsEnvelope(t *testing.T) {
|
||
var gotMethod string
|
||
var gotArgs map[string]any
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
var p callParams
|
||
json.Unmarshal(params, &p)
|
||
gotMethod, gotArgs = p.Method, p.Args
|
||
return fakeResult(map[string]any{
|
||
"activityId": float64(111),
|
||
"lapDTOs": []map[string]any{{"lapIndex": float64(1), "distance": float64(1000)}},
|
||
})
|
||
})
|
||
|
||
splits, err := c.GetActivitySplits(context.Background(), 111)
|
||
if err != nil {
|
||
t.Fatalf("GetActivitySplits: %v", err)
|
||
}
|
||
if gotMethod != "get_activity_splits" || gotArgs["activity_id"] != "111" {
|
||
t.Errorf("method/args = %q/%+v, want get_activity_splits with activity_id=\"111\"", gotMethod, gotArgs)
|
||
}
|
||
if splits.ActivityID != 111 || len(splits.Laps) != 1 || splits.Laps[0].LapIndex != 1 {
|
||
t.Fatalf("splits = %+v", splits)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_GetActivityDetails_ParsesRawTelemetry(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
var p callParams
|
||
json.Unmarshal(params, &p)
|
||
if p.Method != "get_activity_details" || p.Args["activity_id"] != "222" {
|
||
t.Fatalf("unexpected call: %+v", p)
|
||
}
|
||
return fakeResult(map[string]any{
|
||
"activityId": float64(222),
|
||
"metricDescriptors": []map[string]any{{"key": "directHeartRate", "metricsIndex": float64(0)}},
|
||
"activityDetailMetrics": []map[string]any{{"metrics": []any{float64(101)}}},
|
||
})
|
||
})
|
||
|
||
details, err := c.GetActivityDetails(context.Background(), 222)
|
||
if err != nil {
|
||
t.Fatalf("GetActivityDetails: %v", err)
|
||
}
|
||
if details.ActivityID != 222 || len(details.MetricDescriptors) != 1 {
|
||
t.Fatalf("details = %+v", details)
|
||
}
|
||
}
|
||
|
||
func TestSubprocessClient_GetWorkoutByID_ParsesSegments(t *testing.T) {
|
||
c := newFakeWrapperClient(t, func(cmd string, params json.RawMessage) wireResponsePayload {
|
||
var p callParams
|
||
json.Unmarshal(params, &p)
|
||
if p.Method != "get_workout_by_id" || p.Args["workout_id"] != "333" {
|
||
t.Fatalf("unexpected call: %+v", p)
|
||
}
|
||
return fakeResult(map[string]any{
|
||
"workoutId": float64(333),
|
||
"workoutName": "Tempo",
|
||
"workoutSegments": []map[string]any{{"segmentOrder": float64(1), "workoutSteps": []any{}}},
|
||
})
|
||
})
|
||
|
||
workout, err := c.GetWorkoutByID(context.Background(), 333)
|
||
if err != nil {
|
||
t.Fatalf("GetWorkoutByID: %v", err)
|
||
}
|
||
if workout.WorkoutID != 333 || workout.WorkoutName != "Tempo" || len(workout.Segments) != 1 {
|
||
t.Fatalf("workout = %+v", workout)
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run the tests to verify they fail**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -run 'TestSubprocessClient_(GetActivities|GetActivitySplits|GetActivityDetails|GetWorkoutByID)' -v`
|
||
Expected: compile failure — these methods don't exist on `subprocessClient` yet.
|
||
|
||
- [ ] **Step 3: Implement the data-fetch methods**
|
||
|
||
First, add `"strconv"` to `backend/internal/garmin/client.go`'s import block (needed by `strconv.FormatInt` below):
|
||
|
||
```go
|
||
import (
|
||
"bufio"
|
||
"context"
|
||
_ "embed"
|
||
"encoding/json"
|
||
"fmt"
|
||
"io"
|
||
"os"
|
||
"os/exec"
|
||
"strconv"
|
||
"sync"
|
||
)
|
||
```
|
||
|
||
Then append to `backend/internal/garmin/client.go` — this is also where `NewClient` is defined for the first time (deferred from Task 2, since its return type requires `*subprocessClient` to already implement all of `Client`, which is only true once these four methods exist):
|
||
|
||
```go
|
||
var _ Client = (*subprocessClient)(nil)
|
||
|
||
// NewClient builds a Client. The subprocess is not spawned until the first
|
||
// call that needs it (Authenticate, or any data call once authenticated).
|
||
func NewClient(cfg Config) Client {
|
||
return &subprocessClient{cfg: cfg}
|
||
}
|
||
|
||
func (c *subprocessClient) GetActivities(ctx context.Context, startDate, endDate string, limit int) ([]Activity, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return nil, err
|
||
}
|
||
raw, err := c.roundTrip("call", callParams{
|
||
Method: "get_activities_by_date",
|
||
Args: map[string]any{"startdate": startDate, "enddate": endDate},
|
||
})
|
||
if err != nil {
|
||
return nil, err
|
||
}
|
||
|
||
var rawActivities []json.RawMessage
|
||
if err := json.Unmarshal(raw, &rawActivities); err != nil {
|
||
return nil, fmt.Errorf("get_activities_by_date did not return a JSON array (%q): %w", truncate(string(raw), 200), err)
|
||
}
|
||
if limit > 0 && limit < len(rawActivities) {
|
||
rawActivities = rawActivities[:limit]
|
||
}
|
||
|
||
activities := make([]Activity, 0, len(rawActivities))
|
||
for _, raw := range rawActivities {
|
||
var a Activity
|
||
if err := json.Unmarshal(raw, &a); err != nil {
|
||
return nil, fmt.Errorf("parse activity: %w", err)
|
||
}
|
||
a.Raw = raw
|
||
activities = append(activities, a)
|
||
}
|
||
return activities, nil
|
||
}
|
||
|
||
func (c *subprocessClient) GetActivitySplits(ctx context.Context, activityID int64) (ActivitySplits, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return ActivitySplits{}, err
|
||
}
|
||
raw, err := c.roundTrip("call", callParams{
|
||
Method: "get_activity_splits",
|
||
Args: map[string]any{"activity_id": strconv.FormatInt(activityID, 10)},
|
||
})
|
||
if err != nil {
|
||
return ActivitySplits{}, err
|
||
}
|
||
|
||
var envelope struct {
|
||
ActivityID int64 `json:"activityId"`
|
||
Laps []json.RawMessage `json:"lapDTOs"`
|
||
}
|
||
if err := json.Unmarshal(raw, &envelope); err != nil {
|
||
return ActivitySplits{}, fmt.Errorf("get_activity_splits did not return valid JSON (%q): %w", truncate(string(raw), 200), err)
|
||
}
|
||
|
||
splits := ActivitySplits{ActivityID: envelope.ActivityID, Laps: make([]Lap, 0, len(envelope.Laps))}
|
||
for _, raw := range envelope.Laps {
|
||
var l Lap
|
||
if err := json.Unmarshal(raw, &l); err != nil {
|
||
return ActivitySplits{}, fmt.Errorf("parse lap: %w", err)
|
||
}
|
||
l.Raw = raw
|
||
splits.Laps = append(splits.Laps, l)
|
||
}
|
||
return splits, nil
|
||
}
|
||
|
||
func (c *subprocessClient) GetActivityDetails(ctx context.Context, activityID int64) (ActivityDetails, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return ActivityDetails{}, err
|
||
}
|
||
raw, err := c.roundTrip("call", callParams{
|
||
Method: "get_activity_details",
|
||
Args: map[string]any{"activity_id": strconv.FormatInt(activityID, 10)},
|
||
})
|
||
if err != nil {
|
||
return ActivityDetails{}, err
|
||
}
|
||
|
||
var details ActivityDetails
|
||
if err := json.Unmarshal(raw, &details); err != nil {
|
||
return ActivityDetails{}, fmt.Errorf("get_activity_details did not return valid JSON (%q): %w", truncate(string(raw), 200), err)
|
||
}
|
||
details.Raw = json.RawMessage(raw)
|
||
return details, nil
|
||
}
|
||
|
||
func (c *subprocessClient) GetWorkoutByID(ctx context.Context, workoutID int64) (Workout, error) {
|
||
c.mu.Lock()
|
||
defer c.mu.Unlock()
|
||
|
||
if err := c.ensureStarted(); err != nil {
|
||
return Workout{}, err
|
||
}
|
||
raw, err := c.roundTrip("call", callParams{
|
||
Method: "get_workout_by_id",
|
||
Args: map[string]any{"workout_id": strconv.FormatInt(workoutID, 10)},
|
||
})
|
||
if err != nil {
|
||
return Workout{}, err
|
||
}
|
||
|
||
var workout Workout
|
||
if err := json.Unmarshal(raw, &workout); err != nil {
|
||
return Workout{}, fmt.Errorf("get_workout_by_id did not return valid JSON (%q): %w", truncate(string(raw), 200), err)
|
||
}
|
||
workout.Raw = json.RawMessage(raw)
|
||
return workout, nil
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass, then build the whole package**
|
||
|
||
Run: `cd backend && go test ./internal/garmin/... -v`
|
||
Expected: all tests in the package PASS.
|
||
|
||
Run: `cd backend && go build ./internal/garmin/...`
|
||
Expected: builds cleanly — `subprocessClient` now fully implements `Client`.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/internal/garmin/client.go backend/internal/garmin/client_test.go
|
||
git commit -m "$(cat <<'EOF'
|
||
feat(garmin): implement data-fetch methods via generic call dispatch
|
||
|
||
GetActivities/GetActivitySplits/GetActivityDetails/GetWorkoutByID now send
|
||
{"cmd":"call","params":{"method":...,"args":...}} instead of named MCP
|
||
tools. subprocessClient fully implements Client.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 5: Update `internal/config` (drop server-path env var, default the python path)
|
||
|
||
**Files:**
|
||
- Modify: `backend/internal/config/config.go`
|
||
- Modify: `backend/internal/config/config_test.go`
|
||
|
||
**Interfaces:**
|
||
- Consumes: nothing from Tasks 1–4.
|
||
- Produces: `Config.GarminPythonPath` (unchanged field name, new default/required behavior); `Config.GarminServerPath` removed entirely.
|
||
|
||
- [ ] **Step 1: Write the failing tests**
|
||
|
||
In `backend/internal/config/config_test.go`, update `setRequiredEnv` (remove the two now-optional lines):
|
||
|
||
```go
|
||
func setRequiredEnv(t *testing.T) {
|
||
t.Helper()
|
||
t.Setenv("GENIUSRUN_PUBLIC_BASE_URL", "https://geniusrun.example.com")
|
||
t.Setenv("GENIUSRUN_OIDC_ISSUER_URL", "https://keycloak.example.com/realms/myrealm")
|
||
t.Setenv("GENIUSRUN_OIDC_CLIENT_ID", "geniusrun")
|
||
t.Setenv("GENIUSRUN_OIDC_CLIENT_SECRET", "client-secret")
|
||
t.Setenv("GENIUSRUN_SESSION_SECRET", "a-session-secret-that-is-at-least-32-bytes-long")
|
||
}
|
||
```
|
||
|
||
Add two new tests (anywhere in the file, e.g. right after `TestLoad_GarminTokenStoreRootExplicitOverridesDefault`):
|
||
|
||
```go
|
||
func TestLoad_GarminPythonPathDefaultsToPython3(t *testing.T) {
|
||
setRequiredEnv(t)
|
||
t.Setenv("GARMIN_WRAPPER_PYTHON", "")
|
||
|
||
cfg, err := Load()
|
||
if err != nil {
|
||
t.Fatalf("Load: %v", err)
|
||
}
|
||
if cfg.GarminPythonPath != "python3" {
|
||
t.Errorf("GarminPythonPath = %q, want default %q", cfg.GarminPythonPath, "python3")
|
||
}
|
||
}
|
||
|
||
func TestLoad_GarminPythonPathExplicitOverridesDefault(t *testing.T) {
|
||
setRequiredEnv(t)
|
||
t.Setenv("GARMIN_WRAPPER_PYTHON", "/opt/venv/bin/python3")
|
||
|
||
cfg, err := Load()
|
||
if err != nil {
|
||
t.Fatalf("Load: %v", err)
|
||
}
|
||
if cfg.GarminPythonPath != "/opt/venv/bin/python3" {
|
||
t.Errorf("GarminPythonPath = %q, want explicit override", cfg.GarminPythonPath)
|
||
}
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 2: Run the tests to verify they fail**
|
||
|
||
Run: `cd backend && go test ./internal/config/... -v`
|
||
Expected: `TestLoad_GarminPythonPathDefaultsToPython3` fails (`GarminPythonPath` is currently `""`, and `Load()` currently errors out entirely since `MCP_GARMIN_PYTHON`/`MCP_GARMIN_SERVER` are no longer set by the trimmed `setRequiredEnv`) — every other test in the package should also now fail with "MCP_GARMIN_PYTHON is required", confirming the removed env vars were the only thing missing.
|
||
|
||
- [ ] **Step 3: Update `config.go`**
|
||
|
||
In `backend/internal/config/config.go`, replace the two Garmin subprocess-path fields:
|
||
|
||
```go
|
||
// GarminPythonPath is mcp-garmin's venv python executable.
|
||
GarminPythonPath string
|
||
// GarminServerPath is mcp-garmin's server.py.
|
||
GarminServerPath string
|
||
```
|
||
|
||
with:
|
||
|
||
```go
|
||
// GarminPythonPath is the python3 interpreter used to run the embedded
|
||
// Garmin wrapper script (internal/garmin's go:embed'd wrapper.py).
|
||
// Defaults to "python3" resolved via PATH if unset.
|
||
GarminPythonPath string
|
||
```
|
||
|
||
In `Load()`, replace:
|
||
|
||
```go
|
||
GarminPythonPath: os.Getenv("MCP_GARMIN_PYTHON"),
|
||
GarminServerPath: os.Getenv("MCP_GARMIN_SERVER"),
|
||
```
|
||
|
||
with:
|
||
|
||
```go
|
||
GarminPythonPath: getEnvDefault("GARMIN_WRAPPER_PYTHON", "python3"),
|
||
```
|
||
|
||
And delete the two now-obsolete required-field checks entirely:
|
||
|
||
```go
|
||
if cfg.GarminPythonPath == "" {
|
||
return cfg, fmt.Errorf("MCP_GARMIN_PYTHON is required (path to mcp-garmin's venv python)")
|
||
}
|
||
if cfg.GarminServerPath == "" {
|
||
return cfg, fmt.Errorf("MCP_GARMIN_SERVER is required (path to mcp-garmin's server.py)")
|
||
}
|
||
```
|
||
|
||
- [ ] **Step 4: Run the tests to verify they pass**
|
||
|
||
Run: `cd backend && go test ./internal/config/... -v`
|
||
Expected: all tests PASS.
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/internal/config/config.go backend/internal/config/config_test.go
|
||
git commit -m "$(cat <<'EOF'
|
||
feat(config): drop MCP_GARMIN_SERVER, default GARMIN_WRAPPER_PYTHON
|
||
|
||
The wrapper script is embedded in the binary now (internal/garmin), so
|
||
there's no script path left to configure. The interpreter path becomes
|
||
optional, defaulting to python3 on PATH, matching how other optional
|
||
plumbing (e.g. GENIUSRUN_OIDC_REQUIRED_ROLE) is already handled.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 6: Wire up `main.go`/`server.go`, update `start.sh` and `CLAUDE.md`
|
||
|
||
**Files:**
|
||
- Modify: `backend/cmd/geniusrund/main.go`
|
||
- Modify: `backend/internal/api/server.go` (comment only)
|
||
- Modify: `backend/start.sh`
|
||
- Modify: `CLAUDE.md`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `garmin.Config{PythonPath, GarminEmail, GarminPassword, TokenStorePath}` (Task 2), `config.Config.GarminPythonPath` (Task 5).
|
||
- Produces: nothing new — this task only removes stale references to the deleted `garmin.Config.ServerPath`/`config.Config.GarminServerPath` fields and refreshes docs.
|
||
|
||
- [ ] **Step 1: Update `main.go`**
|
||
|
||
In `backend/cmd/geniusrund/main.go`, change:
|
||
|
||
```go
|
||
server := api.NewServer(db, garmin.NewClient, garmin.Config{
|
||
PythonPath: cfg.GarminPythonPath,
|
||
ServerPath: cfg.GarminServerPath,
|
||
TokenStorePath: cfg.GarminTokenStoreRoot,
|
||
}, appsync.Config{
|
||
```
|
||
|
||
to:
|
||
|
||
```go
|
||
server := api.NewServer(db, garmin.NewClient, garmin.Config{
|
||
PythonPath: cfg.GarminPythonPath,
|
||
TokenStorePath: cfg.GarminTokenStoreRoot,
|
||
}, appsync.Config{
|
||
```
|
||
|
||
- [ ] **Step 2: Update the `GarminBase` doc comment in `server.go`**
|
||
|
||
In `backend/internal/api/server.go`, change:
|
||
|
||
```go
|
||
// GarminBase holds the plumbing shared by every user's garmin.Config
|
||
// (subprocess paths + the token-store root directory); only
|
||
// GarminEmail/GarminPassword/TokenStorePath vary per user, filled in by
|
||
// garminFor.
|
||
```
|
||
|
||
to:
|
||
|
||
```go
|
||
// GarminBase holds the plumbing shared by every user's garmin.Config
|
||
// (the python interpreter path + the token-store root directory); only
|
||
// GarminEmail/GarminPassword/TokenStorePath vary per user, filled in by
|
||
// garminFor.
|
||
```
|
||
|
||
- [ ] **Step 3: Update `start.sh`**
|
||
|
||
In `backend/start.sh`, remove these two lines:
|
||
|
||
```bash
|
||
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}"
|
||
```
|
||
|
||
and replace them with a comment (no export needed — `config.Load()` already defaults it):
|
||
|
||
```bash
|
||
# 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.
|
||
```
|
||
|
||
- [ ] **Step 4: Update `CLAUDE.md`**
|
||
|
||
Replace the "Run the server" bullet under `## Commands`:
|
||
|
||
```markdown
|
||
- 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.
|
||
```
|
||
|
||
with:
|
||
|
||
```markdown
|
||
- 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).
|
||
```
|
||
|
||
Replace the `internal/garmin/` line under `## Repo layout`:
|
||
|
||
```markdown
|
||
internal/garmin/ MCP client wrapper (auth, get_activities, get_activity_splits, get_activity_details, get_workout_by_id)
|
||
```
|
||
|
||
with:
|
||
|
||
```markdown
|
||
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)
|
||
```
|
||
|
||
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:
|
||
|
||
- **`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.
|
||
```
|
||
|
||
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:
|
||
|
||
- **`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.
|
||
```
|
||
|
||
- [ ] **Step 5: Verify the whole backend builds**
|
||
|
||
Run: `cd backend && go build ./... && go vet ./...`
|
||
Expected: no errors.
|
||
|
||
- [ ] **Step 6: Commit**
|
||
|
||
```bash
|
||
git add backend/cmd/geniusrund/main.go backend/internal/api/server.go backend/start.sh CLAUDE.md
|
||
git commit -m "$(cat <<'EOF'
|
||
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>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
### Task 7: Remove the `mcp-go` dependency and do a full-repo verification pass
|
||
|
||
**Files:**
|
||
- Modify: `backend/go.mod`, `backend/go.sum` (via `go mod tidy`)
|
||
|
||
**Interfaces:**
|
||
- Consumes: everything from Tasks 1–6.
|
||
- Produces: nothing new — this is the final cleanup + verification task.
|
||
|
||
- [ ] **Step 1: Confirm nothing else references `mcp-go`**
|
||
|
||
Run: `cd backend && grep -rn "mark3labs/mcp-go" --include=*.go .`
|
||
Expected: no output (Tasks 2–4 already removed the only import, in `client.go`).
|
||
|
||
- [ ] **Step 2: Tidy the module**
|
||
|
||
Run: `cd backend && go mod tidy`
|
||
Expected: `github.com/mark3labs/mcp-go` and its now-unreferenced indirect dependencies (`google/jsonschema-go`, `yosida95/uritemplate`, `santhosh-tekuri/jsonschema`, `spf13/cast`, etc. — whichever `go mod tidy` determines are no longer needed) are removed from `go.mod`/`go.sum`.
|
||
|
||
- [ ] **Step 3: Full build, vet, format, and test pass**
|
||
|
||
Run, in order:
|
||
```bash
|
||
cd backend
|
||
gofmt -l .
|
||
go build ./...
|
||
go vet ./...
|
||
go test ./...
|
||
```
|
||
Expected: `gofmt -l .` prints nothing; `go build`/`go vet` produce no errors; `go test ./...` passes for every package, including `internal/garmin`, `internal/config`, `internal/api`, `internal/sync` (unchanged behavior, since `mock.Client` and the `Client` interface didn't change).
|
||
|
||
- [ ] **Step 4: Run the Python test suite once more for good measure**
|
||
|
||
Run: `cd backend/internal/garmin/pyscript && .venv/bin/pytest -v`
|
||
Expected: all PASS (should be unchanged since Task 1, included here as a final end-to-end sanity check now that the Go side fully exercises the same protocol).
|
||
|
||
- [ ] **Step 5: Commit**
|
||
|
||
```bash
|
||
git add backend/go.mod backend/go.sum
|
||
git commit -m "$(cat <<'EOF'
|
||
chore: drop the mcp-go dependency
|
||
|
||
Nothing in the backend speaks MCP anymore -- internal/garmin talks to its
|
||
embedded Python wrapper over plain JSON-lines instead.
|
||
|
||
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
EOF
|
||
)"
|
||
```
|
||
|
||
---
|
||
|
||
## Follow-up note (not a task in this plan)
|
||
|
||
The standalone `mcp-garmin` repo (`/Users/kriss/Dev/scm/scm.vilanet.fr/kriss/mcp-garmin`) is superseded by `backend/internal/garmin/pyscript/` and can be archived once this plan lands — that's a separate-repo action for you to do directly (e.g. marking it archived on your SCM host), not something this plan's tasks touch.
|