diff --git a/AGENTS.md b/AGENTS.md index 6955241..30e34d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,7 @@ Remaining before 1.0: - Document types `02`/`03`/`04`/`11`-`14` (only `01` Invoice is modelled). - Live sandbox verification against LHDN preprod. - `tests/unit/test_auth.py::test_token_manager_refresh_margin` is **vacuous**: it never calls `get_token()`, so `is_valid()` returns False only because no token was ever acquired. Proven by re-running it with `expires_in=3600` (far outside the 60s margin) — still passes, 0 calls to the token endpoint. It would pass with the refresh-margin logic deleted. The async twin was strengthened in the Phase 6b coverage PR; fix the sync one the same way (two responses, two `get_token()` calls, assert re-acquisition). -- `TokenManager.is_valid()` is a **method** but `AsyncTokenManager.is_valid` is a **property** — a real sync/async divergence that breaks the "one-for-one mirror" contract this file and the README both claim. Porting sync code to async hits `TypeError: 'bool' object is not callable`. Found while writing the async auth tests; deliberately left for its own PR. +- `TokenManager` creates its own `httpx.Client` when none is injected but exposes no `close()`, while `AsyncTokenManager` tracks `_owns_client` and has `aclose()`. **Not a live leak** — both clients pass their own `httpx` client in, so the manager never builds one on the normal path; it only bites a bare `TokenManager()`, which is not exported. Low priority. ## CODE_STATE - `src/myinvois/codes/__init__.py` implements curated `StrEnum(_EnumLookupMixin)` tables + `_CodeTable` loader instances; `src/myinvois/codes/_data/*.json` 8 tables = 3,637 rows. Re-extract via `uv run python scripts/extract_codes.py`. @@ -610,6 +610,13 @@ HTTP-status -> exception mapping. `_async_auth.py` 76% -> 92%, The lock test is verified to be able to fail: neutering `AsyncTokenManager._lock` with a no-op async context manager turns 1 token request into 10. +## SYNC/ASYNC MIRROR CONTRACT — enforced by a test +`TokenManager.is_valid` was a **method** while `AsyncTokenManager.is_valid` was a **property**, so porting sync code to async raised `TypeError: 'bool' object is not callable`. Neither suite caught it because each only exercised its own side. Resolved by making **both properties** — consistent with `access_token` and `token`, which were already properties on both, and chosen over the reverse because nothing is published yet so there are no callers to break. + +`tests/unit/test_async_auth.py::test_managers_expose_the_same_public_surface` now compares the two classes' public members *and their kinds* (`property` vs `function`) via `inspect.getattr_static`, so any future drift fails the build. `aclose` is the single sanctioned async-only member. Verified to catch the original bug: reverting `auth.py` makes it fail with `{'is_valid': ('function', 'property')}`. + +**When adding a member to either manager, add it to both** — or add it to the test's sanctioned-difference set with a reason. + ## PHASE 6b — Polish + publish (partially DONE) 1. **CI** — **DONE.** `.github/workflows/ci.yml` with `lint` / `test` (3.11-3.13 matrix) / `package` jobs. See the GITHUB section. Adding it surfaced 4 pre-existing `ruff format` failures — see the FORMATTING DRIFT section. diff --git a/src/myinvois/auth.py b/src/myinvois/auth.py index bccc178..1c34a94 100644 --- a/src/myinvois/auth.py +++ b/src/myinvois/auth.py @@ -113,6 +113,7 @@ def __init__( # ----- introspection ------------------------------------------------- + @property def is_valid(self) -> bool: tok = self._stored.value if tok is None: diff --git a/tests/unit/test_async_auth.py b/tests/unit/test_async_auth.py index 2afcc9e..2e1cadc 100644 --- a/tests/unit/test_async_auth.py +++ b/tests/unit/test_async_auth.py @@ -20,7 +20,7 @@ import pytest from myinvois._async_auth import AsyncTokenManager -from myinvois.auth import OAuth2Token +from myinvois.auth import OAuth2Token, TokenManager from myinvois.config import Environment, base_identity_url from myinvois.exceptions import AuthenticationError, MyInvoisError @@ -67,6 +67,38 @@ def _expire_now(mgr: AsyncTokenManager) -> None: ) +# ===== sync/async parity ===== + + +def test_managers_expose_the_same_public_surface() -> None: + """Pin the "one-for-one mirror" contract that AGENTS.md and the README claim. + + ``is_valid`` was previously a method on ``TokenManager`` but a property on + ``AsyncTokenManager``, so porting sync code to async raised + ``TypeError: 'bool' object is not callable`` at runtime. Nothing caught it + because each suite only exercised its own side. + + ``aclose`` is the one sanctioned difference: it has no sync counterpart. + """ + import inspect + + def surface(cls: type) -> dict[str, str]: + return { + name: type(inspect.getattr_static(cls, name)).__name__ + for name in dir(cls) + if not name.startswith("_") + } + + sync, async_ = surface(TokenManager), surface(AsyncTokenManager) + + assert set(async_) - set(sync) == {"aclose"} + assert set(sync) - set(async_) == set() + + shared = set(sync) & set(async_) + mismatched = {name: (sync[name], async_[name]) for name in shared if sync[name] != async_[name]} + assert not mismatched, f"sync/async member kind drift: {mismatched}" + + # ===== caching and refresh ===== diff --git a/tests/unit/test_auth.py b/tests/unit/test_auth.py index 306fb6f..be1c583 100644 --- a/tests/unit/test_auth.py +++ b/tests/unit/test_auth.py @@ -72,7 +72,7 @@ def test_token_manager_acquires_token(respx_mock: Any, token_url: str, mgr: Toke tok = mgr.get_token() assert tok.access_token == "TOK-1" - assert mgr.is_valid() is True + assert mgr.is_valid is True def test_token_manager_caches_until_expiry( @@ -115,7 +115,7 @@ def test_token_manager_refresh_margin(respx_mock: Any, token_url: str, mgr: Toke return_value=httpx.Response(200, json=_token_response("TOK", expires_in=30)) ) - assert mgr.is_valid() is False + assert mgr.is_valid is False def test_token_manager_raises_on_auth_failure( @@ -145,7 +145,7 @@ def test_new_token_manager_has_no_token(token_url: str) -> None: fresh = TokenManager( client_id="cid", client_secret="csecret", token_url=token_url, scope="InvoicingAPI" ) - assert fresh.is_valid() is False + assert fresh.is_valid is False assert fresh.access_token is None assert fresh.token is None @@ -160,4 +160,4 @@ def test_invalidate_drops_cached_token(respx_mock: Any, token_url: str, mgr: Tok mgr.invalidate() assert mgr.access_token is None - assert mgr.is_valid() is False + assert mgr.is_valid is False