Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/myinvois/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def __init__(

# ----- introspection -------------------------------------------------

@property
def is_valid(self) -> bool:
tok = self._stored.value
if tok is None:
Expand Down
34 changes: 33 additions & 1 deletion tests/unit/test_async_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 =====


Expand Down
8 changes: 4 additions & 4 deletions tests/unit/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand All @@ -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
Loading