fix: bound startup-login timeout, refresh stale SKILL.md and doc comments

_startup_login() ran garminconnect's login synchronously with no timeout
before main() ever started reading stdin, so a slow/rate-limited Garmin
login could wedge a user's whole subprocess before it became responsive.
Give it the same background-thread + bounded-10s-timeout shape
_handle_authenticate already uses, with tests for both the fast-success
and timeout paths.

Also refreshes .claude/skills/geniusrun-dev/SKILL.md (still describing
the retired mcp-garmin MCP architecture) and three stale doc comments
(garmin.AuthStatus, config.GarminTokenStoreRoot, mock package doc) left
over from the direct-wrapper migration.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

.claude/skills/geniusrun-dev/SKILL.md

@@ -1,13 +1,13 @@
 ---
 name: geniusrun-dev
-description: Use when working on the geniusrun repo (backend Go services, classification rule engine, mcp-garmin integration, or frontend) to stay consistent with established conventions.
+description: Use when working on the geniusrun repo (backend Go services, classification rule engine, Garmin integration, or frontend) to stay consistent with established conventions.
 ---
 
 # geniusrun-dev
 
 ## Project overview
 
-geniusrun is a personal web app that pulls running activities from Garmin Connect (via the `mcp-garmin` MCP server), classifies each run into a user-defined "workout kind" (Easy, Tempo, Threshold, Interval, ...), and charts progression over time per kind. Ambiguous runs (matching zero, multiple, or only weakly one kind) go to a manual review queue instead of being silently misclassified.
+geniusrun is a personal web app that pulls running activities from Garmin Connect (via an embedded Python wrapper around `garminconnect`, spoken to over a JSON-lines subprocess protocol — see the Garmin integration section below), classifies each run into a user-defined "workout kind" (Easy, Tempo, Threshold, Interval, ...), and charts progression over time per kind. Ambiguous runs (matching zero, multiple, or only weakly one kind) go to a manual review queue instead of being silently misclassified.
 
 **MVP scope boundary**: sync + classification + review queue + progression charts. A future training-recommendation engine (analyzing aerobic/anaerobic training-effect balance across kinds to suggest what to train next) is explicitly deferred — the schema stores the metrics it would need, but nothing consumes them yet.
 
@@ -17,7 +17,7 @@ geniusrun is a personal web app that pulls running activities from Garmin Connec
 backend/
   cmd/geniusrund/     main server entrypoint
   cmd/seedsample/     inserts synthetic data for frontend dev/demoing without live Garmin creds
-  internal/garmin/     Garmin Connect client: spawns an embedded Python wrapper over a JSON-lines subprocess protocol
+  internal/garmin/     Garmin Connect client: spawns an embedded Python wrapper (pyscript/wrapper.py, garminconnect) over a JSON-lines subprocess protocol (auth, get_activities, get_activity_splits, get_activity_details, get_workout_by_id)
   internal/garmin/mock/  fake Client for tests
   internal/classify/   pure rule engine (condition tree eval, scoring, interval detection, HR drift/recovery)
   internal/store/      SQLite layer + embedded migrations
@@ -53,16 +53,16 @@ Each `workout_kinds.rule_json` is a recursive AND/OR condition tree (`internal/c
 - **Interval detection** (`classify.DetectIntervalPattern`) trusts Garmin's own per-lap `IntensityType` tagging (ACTIVE vs REST/RECOVERY/WARMUP/COOLDOWN) rather than inferring it from pace variance — the device already knows which laps were work vs rest when the activity was recorded as a structured workout.
 - **HR drift/recovery** (`classify.HRDrift`/`HRRecovery`): linear regression of heart rate vs elapsed time within a lap's sample window. Drift = rising HR during an active lap (cardiac drift). Recovery = HR decay rate during a rest lap, sign-flipped so positive = good recovery.
 
-## 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`) 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 (default `~/.garth`) 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.
+- **`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.
 - **`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()`** (added to mcp-garmin, wraps `garminconnect`'s existing `get_activity_splits`) is the one that returns actual lap/split summaries (`lapDTOs`).
+- **`get_activity_splits`** returns the actual lap/split summaries (`lapDTOs`).
 
 ## Data model conventions
 
@@ -74,7 +74,7 @@ Each `workout_kinds.rule_json` is a recursive AND/OR condition tree (`internal/c
 
 ## Dev workflow
 
-- Backend: `cd backend && go run ./cmd/geniusrund` (needs `MCP_GARMIN_PYTHON`/`MCP_GARMIN_SERVER` env vars for the mcp-garmin subprocess paths; see `internal/config/config.go` for all knobs). Garmin credentials are **not** env vars — they live in the `profile` singleton row (`internal/store.Profile`), set via the frontend's Profile page or directly through `PUT /api/profile`.
+- Backend: `cd backend && go run ./cmd/geniusrund`. 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) — see `internal/config/config.go` for all knobs. Garmin credentials are **not** env vars — they live in the `profile` singleton row (`internal/store.Profile`), set via the frontend's Profile page or directly through `PUT /api/profile`.
 - Frontend: `cd frontend && npm run dev` (set `VITE_API_BASE_URL` if the backend isn't on `localhost:8080`).
 - No live Garmin account needed for frontend/UI work: `go run ./cmd/seedsample -db /tmp/sample.db` seeds realistic activities/laps/kinds and runs them through the real classification engine, then point `geniusrund` at that DB.
 - `internal/garmin/mock` provides a fake `Client` for tests that need to exercise `internal/sync`/`internal/api` without a live subprocess.

backend/internal/config/config.go

@@ -27,7 +27,7 @@ type Config struct {
 	// Defaults to "python3" resolved via PATH if unset.
 	GarminPythonPath string
 	// GarminTokenStoreRoot is the root directory under which each user's
-	// mcp-garmin session cache lives (one subdirectory per user id, e.g.
+	// Garmin session cache lives (one subdirectory per user id, e.g.
 	// "<root>/3"). Read from the GARMIN_TOKENSTORE env var; if unset, it
 	// defaults to a "garmin-tokenstores" directory next to DBPath so every
 	// deployment gets per-user isolation automatically -- multi-tenant

backend/internal/garmin/mock/mock.go

@@ -1,5 +1,5 @@
 // Package mock provides a fake garmin.Client for tests and frontend/dev
-// work without a live Garmin account or the mcp-garmin subprocess.
+// work without a live Garmin account or the wrapper subprocess.
 package mock
 
 import (

backend/internal/garmin/pyscript/tests/test_wrapper.py

@@ -124,3 +124,35 @@ 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"]
+
+
+def test_startup_login_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()
+            wrapper._startup_login()
+    assert wrapper._auth_state == "authenticated"
+
+
+def test_startup_login_missing_credentials_returns_immediately():
+    with patch.dict("os.environ", {}, clear=False):
+        os.environ.pop("GARMIN_EMAIL", None)
+        os.environ.pop("GARMIN_PASSWORD", None)
+        wrapper._auth_state = "unauthenticated"
+        wrapper._startup_login()
+    assert wrapper._auth_state == "unauthenticated"
+    assert wrapper._client is None
+
+
+def test_startup_login_times_out_without_blocking():
+    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):
+                # Should return promptly (bounded by the 10s timeout passed to
+                # queue.get, which is mocked here to raise immediately) rather
+                # than blocking main()'s stdin dispatch loop from starting.
+                wrapper._startup_login()
+    assert wrapper._auth_state == "unauthenticated"

backend/internal/garmin/pyscript/wrapper.py

@@ -33,18 +33,45 @@ def _prompt_mfa():
 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."""
+    syncs that never call the explicit authenticate command.
+
+    Bounded to the same 10s timeout as _handle_authenticate: the actual
+    login runs on a background daemon thread, and this function waits up
+    to 10s for it before returning either way. This runs before main()
+    enters its stdin dispatch loop, so a slow/rate-limited Garmin login
+    must never block indefinitely here -- if it times out, the thread
+    keeps running and will update _auth_state whenever it eventually
+    finishes (success or failure), same as today, just without wedging
+    the whole subprocess unresponsive in the meantime."""
     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)
+
+    def _do_login():
+        try:
             _client.login(tokenstore=TOKENSTORE)
-        _auth_state = "authenticated"
+            _login_result_queue.put(("success", None))
         except Exception as exc:
-        _debug(f"startup tokenstore login failed: {exc}")
+            _login_result_queue.put(("error", str(exc)))
+
+    threading.Thread(target=_do_login, daemon=True).start()
+
+    try:
+        status, err = _login_result_queue.get(timeout=10)
+        if status == "success":
+            _auth_state = "authenticated"
+        else:
+            _debug(f"startup tokenstore login failed: {err}")
+            _auth_state = "unauthenticated"
+    except queue.Empty:
+        _debug(
+            "startup tokenstore login hit the 10s timeout -- still running in the "
+            "background and will update auth state whenever it finishes"
+        )
         _auth_state = "unauthenticated"
 
 

backend/internal/garmin/types.go

@@ -3,8 +3,8 @@ package garmin
 import "encoding/json"
 
 // AuthStatus is the outcome of an authenticate()/complete_mfa() call.
-// mcp-garmin's tools return a plain human-readable string rather than a
+// wrapper.py returns a structured {"status", "message"} JSON object, which
-// structured status, so the client pattern-matches known phrases into this.
+// the client maps directly into this rather than pattern-matching strings.
 type AuthStatus int
 
 const (