Conversation
| Two shapes coexist; match the file you're editing. | ||
|
|
||
| - **Module-level functions** — `test_server_client.py` and `test_dpop_auth.py` | ||
| - **Grouping classes** — `test_mfa_client.py`, `test_my_account_client.py` use `TestMfaClientConstructor`, |
There was a problem hiding this comment.
test_my_account_client.py is grouped here with test_mfa_client.py as using grouping classes, but it doesn't have any Test* classes, it's 68 module-level test functions with # === banners. The two class names on the next line (TestMfaClientConstructor, TestDomainResolution) only exist in test_mfa_client.py.
Shall we move test_my_account_client.py down to the module-level-functions bullet, so test_mfa_client.py is the only grouping-class example?
| |-------|---------|---------| | ||
| | `<feature>-support` | `passkey-support`, `dpop-support`, `myaccount-support` | a new capability | | ||
| | `SDK-<ticket>-<slug>` | `SDK-8833-organisations-support` | ticket-tracked work | | ||
| | `fix/<slug>` | `fix/cte-model-validators` | bug fixes | |
There was a problem hiding this comment.
The fix/cte-model-validators example doesn't match any branch in this repo. The only fix/ branches that ever existed are things like fix/mfa-breaking-changes. Every other example in this table (passkey-support, SDK-8833-organisations-support, feat/cte-delegation, docs/sync-2026-06-30) points at a real PR, so this one stands out.
Can we swap it for a real one, fix/mfa-breaking-changes (#104) is an actual bug-fix branch?
| ## 4. Never construct `httpx.AsyncClient` directly | ||
|
|
||
| `ServerClient._get_http_client()` and `MyAccountClient._get_http_client()` merge | ||
| `self._telemetry_headers` into every request. A hand-rolled client silently drops the `Auth0-Client` |
There was a problem hiding this comment.
MyAccountClient._get_http_client() merges self._headers, not self._telemetry_headers (that attribute only exists on ServerClient). It still works because ServerClient builds the MyAccount client with headers=self._telemetry_headers, so the header does get attached, but the name here doesn't match the code an agent would open to.
| reformatted, including `server_client.py`'s test file and `helpers.py`. Running `ruff format .` | ||
| repo-wide produces a thousand-line diff unrelated to your change. Format only what you touched. | ||
|
|
||
| ## 3. Async tests silently pass if you forget `@pytest.mark.asyncio` |
There was a problem hiding this comment.
I ran this in the repo's venv (pytest 7.4.4, pytest-asyncio 0.23.8, strict mode): an unmarked async test isn't reported as passing, it's skipped with a PytestUnhandledCoroutineWarning and shows up as s / "skipped" in the summary. testing.md already describes it correctly as skipped; it's this file and CLAUDE.md that say "passes."
The heading and the "if a new async test passes on the first run, check for the marker" tip point an agent at the wrong symptom, it'll be looking for a pass when the real signal is a skip. Can we reword both to "skipped (with a warning), never executed"?
| ### ✅ Always Do | ||
|
|
||
| - Run `poetry run pytest` and `poetry run ruff check .` before committing — both gate CI. | ||
| - Add tests for new functionality, and mark async tests `@pytest.mark.asyncio` (there is no `asyncio_mode = auto`; an unmarked async test never runs but reports as passing). |
There was a problem hiding this comment.
Same thing as the async pitfall: an unmarked async test is skipped with a warning, not "reports as passing." I confirmed it by running one in the repo's venv. Shall we bring this in line with testing.md's "collected and skipped, never runs"?
| | Doc | What it covers | Status | | ||
| |-----|----------------|--------| | ||
| | `README.md` | Install, `ServerClient` construction, interactive login, custom token exchange, MCD, session expiry, passkeys, My Account, DPoP — each a short section that links out to `examples/` | present | | ||
| | `EXAMPLES.md` | — | **❌ missing.** This repo puts per-feature guides in `examples/*.md` instead. Add a sample to the matching `examples/` guide; only create `EXAMPLES.md` if the team decides to consolidate. | |
There was a problem hiding this comment.
The red-X "❌ missing" reads like an actionable gap, even though the same cell then says the repo uses examples/*.md on purpose. Might be clearer as "not used by design" so an agent doesn't try to create EXAMPLES.md.
Changes
Adds AI-agent guidance files so Claude Code, Codex CLI, Gemini CLI, and any other tool that reads
AGENTS.mdload this repo's actual conventions instead of inferring them. Generated by theteam-sdks:generating-claude-mdskill (v1.16.0).CLAUDE.mdAGENTS.md@./CLAUDE.mdper the agent-files spec — no duplicated contentreferences/code-style.md.ruff.tomlrule sets, naming table, good/bad method examples, dominant patternsreferences/testing.md@pytest.mark.asynciorequirement, mocking approachreferences/commands.mdreferences/pitfalls.mdstore_optionsthreading, …)references/git-workflow.mdreferences/docs-update.mdWhy
references/and notdocs/..gitignore:25ignoresdocsas buildoutput — files placed there would not be tracked.
References
team-sdks:generating-claude-mdv1.16.0 (Mode 1, then re-validatedin Mode 2 with no content changes)
To review: read
CLAUDE.mdend to end — it should contain nothing you'd disagreewith as a maintainer — then spot-check any reference file against its source of
truth. The boundaries encode team judgment; the Ask-First list is the place to
push back.
lint and the full suite pass locally; CI covers 3.9/3.10/3.11/3.12
The one real effect is on AI agents working in this repo: they will follow the
documented boundaries. An incorrect boundary would misdirect an agent, which is
why the boundaries warrant a maintainer's read rather than a rubber stamp.
Checklist