From 5b99e8afcd534db16a5c8197e792a8191b9b9f86 Mon Sep 17 00:00:00 2001
From: ak2k <19240940+ak2k@users.noreply.github.com>
Date: Fri, 15 May 2026 07:51:08 -0400
Subject: [PATCH] =?UTF-8?q?Pin=20the=20401=20=E2=86=92=20refresh=20?=
 =?UTF-8?q?=E2=86=92=20retry=20path=20in=20PPClient.=5Frequest?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The retry only fires when a token expires mid-session, so it's exercised
by chance rather than by design. When it regresses, the failure mode is
a cascade: every PP request returns 401, no awards render, callers see
"no award found" for the entire session.

Three tests, all driven by httpx.MockTransport + a monkeypatched
refresh_tokens:

  1. happy path — 401 → refresh fires once → retry uses the new
     access token → 200 surfaces to caller
  2. non-401 — refresh must NOT fire; response passes through
  3. double-401 — retry-once semantics: refresh runs once, no third
     request, second 401 surfaces (we don't loop forever)

Closes work-rdus.
---
 tests/pp/test_client_request_retry.py | 161 ++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100644 tests/pp/test_client_request_retry.py

diff --git a/tests/pp/test_client_request_retry.py b/tests/pp/test_client_request_retry.py
new file mode 100644
index 0000000..cf8e1dd
--- /dev/null
+++ b/tests/pp/test_client_request_retry.py
@@ -0,0 +1,161 @@
+# pyright: reportPrivateUsage=false
+"""Tests for PPClient._request's 401 -> refresh -> retry behaviour.
+
+The retry happens deep inside the async request path; in production it
+triggers only when a token expires mid-session, so it's exercised by
+chance rather than by design. These tests pin the behaviour with a
+MockTransport so regressions surface immediately."""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import anyio
+import httpx
+
+from flight_cli.pp import client as client_mod
+from flight_cli.pp.auth import Tokens
+from flight_cli.pp.client import API_BASE, PPClient
+
+if TYPE_CHECKING:
+    from collections.abc import Callable
+
+    import pytest
+
+
+def _tokens(access: str = "OLD_TOKEN") -> Tokens:
+    return Tokens(
+        access_token=access,
+        refresh_token="REFRESH",  # noqa: S106 — dummy test value, not a real credential
+        expires_at=9999999999,
+        user_email="test@example.com",
+    )
+
+
+def _recording_refresh_stub(
+    calls: list[Tokens], new_access: str = "NEW_TOKEN"
+) -> Callable[[Tokens], Tokens]:
+    """Build a typed monkeypatch replacement for `refresh_tokens` that
+    records each invocation into `calls` and returns a fresh-looking Tokens."""
+
+    def stub(t: Tokens) -> Tokens:
+        calls.append(t)
+        return _tokens(access=new_access)
+
+    return stub
+
+
+def _scripted_handler(
+    responses: list[httpx.Response],
+) -> tuple[Callable[[httpx.Request], httpx.Response], list[httpx.Request]]:
+    """Hand back (i+1)-th response on the (i+1)-th call; record every request."""
+    captured: list[httpx.Request] = []
+    state = {"i": 0}
+
+    def handler(req: httpx.Request) -> httpx.Response:
+        captured.append(req)
+        idx = state["i"]
+        state["i"] += 1
+        return responses[idx]
+
+    return handler, captured
+
+
+def _client_with_transport(tokens: Tokens, transport: httpx.MockTransport) -> PPClient:
+    """Construct a PPClient and swap its httpx client for one backed by the mock."""
+    pp = PPClient(tokens)
+    # Replace the real AsyncClient with one wired through the mock transport.
+    # The constructor's client has nothing to clean up (no I/O happened yet);
+    # tearing it down would force an event-loop dance in a sync fixture.
+    pp._client = httpx.AsyncClient(
+        base_url=API_BASE, transport=transport, headers=pp._client.headers
+    )
+    return pp
+
+
+# ─────────────────────────────────── 401 → refresh → retry ─────────────────────────────
+
+
+def test_401_refresh_retry_returns_second_response(monkeypatch: pytest.MonkeyPatch) -> None:
+    """The full happy path: 401 → refresh_tokens called once → retry → 200."""
+    refresh_calls: list[Tokens] = []
+
+    def _stub_refresh(t: Tokens) -> Tokens:
+        refresh_calls.append(t)
+        return _tokens(access="NEW_TOKEN")
+
+    monkeypatch.setattr(client_mod, "refresh_tokens", _stub_refresh)
+
+    handler, captured = _scripted_handler(
+        [
+            httpx.Response(401, text="expired"),
+            httpx.Response(200, json={"ok": True}),
+        ]
+    )
+    pp = _client_with_transport(_tokens(), httpx.MockTransport(handler))
+
+    async def go() -> httpx.Response:
+        try:
+            return await pp._request("GET", "/api/pricing-info")
+        finally:
+            await pp.aclose()
+
+    r = anyio.run(go)
+
+    assert r.status_code == 200
+    assert r.json() == {"ok": True}
+    assert len(refresh_calls) == 1, "refresh_tokens should be called exactly once"
+    assert len(captured) == 2, "request should be sent twice (initial + retry)"
+    assert captured[0].headers["authorization"] == "Bearer OLD_TOKEN"
+    assert captured[1].headers["authorization"] == "Bearer NEW_TOKEN", (
+        "retry must use the refreshed access token"
+    )
+
+
+def test_non_401_does_not_refresh(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Any non-401 status passes through unmodified; refresh must not fire."""
+    refresh_calls: list[Tokens] = []
+    monkeypatch.setattr(client_mod, "refresh_tokens", _recording_refresh_stub(refresh_calls))
+
+    handler, captured = _scripted_handler([httpx.Response(500, text="server error")])
+    pp = _client_with_transport(_tokens(), httpx.MockTransport(handler))
+
+    async def go() -> httpx.Response:
+        try:
+            return await pp._request("GET", "/api/pricing-info")
+        finally:
+            await pp.aclose()
+
+    r = anyio.run(go)
+
+    assert r.status_code == 500
+    assert refresh_calls == [], "non-401 must not trigger refresh"
+    assert len(captured) == 1, "no retry on non-401"
+
+
+def test_double_401_returns_second_response_without_third_attempt(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """If the retry also 401s, we surface that response instead of looping forever."""
+    refresh_calls: list[Tokens] = []
+    monkeypatch.setattr(client_mod, "refresh_tokens", _recording_refresh_stub(refresh_calls))
+
+    handler, captured = _scripted_handler(
+        [
+            httpx.Response(401, text="expired"),
+            httpx.Response(401, text="still expired"),
+        ]
+    )
+    pp = _client_with_transport(_tokens(), httpx.MockTransport(handler))
+
+    async def go() -> httpx.Response:
+        try:
+            return await pp._request("GET", "/api/pricing-info")
+        finally:
+            await pp.aclose()
+
+    r = anyio.run(go)
+
+    assert r.status_code == 401
+    assert len(refresh_calls) == 1, "refresh runs once even when retry also 401s"
+    assert len(captured) == 2, "retry-once semantics: no third request"