diff --git a/src/cronometer_api_mcp/client.py b/src/cronometer_api_mcp/client.py index 07c73c0..38c24c0 100644 --- a/src/cronometer_api_mcp/client.py +++ b/src/cronometer_api_mcp/client.py @@ -269,6 +269,13 @@ def _ensure_auth(self) -> None: if self._token is None: self.login() + @property + def user_id(self) -> int: + """Authenticated user id; logs in first if needed (#30/#31).""" + self._ensure_auth() + assert self._user_id is not None + return self._user_id + def _auth_block(self) -> dict: return { "userId": self._user_id, @@ -281,7 +288,11 @@ def _auth_block(self) -> dict: # ------------------------------------------------------------------ def _request(self, endpoint: str, payload: dict, *, _retried: bool = False) -> dict: - """Send a v2 POST request with JSON auth block. Re-authenticates once on failure.""" + """Send a v2 POST request with JSON auth block. Re-authenticates once on failure. + + Callers must build payloads from authenticated state (read identity via + self.user_id, not self._user_id) so the retry can safely re-send the dict. + """ self._ensure_auth() payload["auth"] = self._auth_block() @@ -343,7 +354,7 @@ def _request_v3( """ self._ensure_auth() - url = f"/api/v3/user/{self._user_id}{path}" + url = f"/api/v3/user/{self.user_id}{path}" logger.debug("Cronometer v3 request: %s %s", method, url) resp = self._http.request( @@ -630,7 +641,7 @@ def add_serving( "time": time_str, "offset": None, "source": None, - "userId": self._user_id, + "userId": self.user_id, "servingId": None, "type": "Serving", "foodId": food_id, diff --git a/tests/test_client.py b/tests/test_client.py index c570d27..80a9337 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -123,6 +123,59 @@ def test_stale_cache_file_is_removed_on_failure(tmp_path): assert not session_path.exists() +# --------------------------------------------------------------------------- +# Cold-start auth ordering (issues #30 / #31) +# +# On a client that hasn't logged in yet, add_serving() must not embed a null +# userId: reading identity has to trigger login first, so the payload is +# correct on the first attempt and the retry never re-sends stale state. +# --------------------------------------------------------------------------- + + +def make_cold_client(tmp_path: Path, responses: list[dict]): + """Like make_client but starts unauthenticated and records posted payloads.""" + client = CronometerClient(session_path=tmp_path / "session.json") + client._user_id = None + client._token = None + + state = {"login": 0, "post": 0, "payloads": []} + + def fake_login() -> None: + state["login"] += 1 + client._user_id = 42 + client._token = f"FRESH_TOKEN_{state['login']}" + + def fake_post(endpoint, json=None): + state["payloads"].append(json) + idx = state["post"] + state["post"] += 1 + body = responses[min(idx, len(responses) - 1)] + return FakeResp(body) + + client.login = fake_login # type: ignore[method-assign] + client._http.post = fake_post # type: ignore[method-assign] + return client, state + + +def test_add_serving_cold_start_embeds_real_user_id(tmp_path): + """First write on a cold client logs in once and sends the real userId.""" + client, state = make_cold_client(tmp_path, [{"result": "SUCCESS", "id": 7}]) + + client.add_serving(food_id=1, measure_id=0, grams=100.0) + + assert state["login"] == 1 + assert state["post"] == 1 # no stale-payload double failure (#31) + assert state["payloads"][0]["serving"]["userId"] == 42 + + +def test_user_id_property_triggers_login(tmp_path): + """Reading user_id on a cold client authenticates and returns the real id.""" + client, state = make_cold_client(tmp_path, [{"result": "SUCCESS"}]) + + assert client.user_id == 42 + assert state["login"] == 1 + + # --------------------------------------------------------------------------- # Diary enrichment (get_food_log food names / per-entry nutrients) # ---------------------------------------------------------------------------