GeezerVison

No issues found across 6 files

_{Re-trigger cubic}

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 src/mcp/client/auth/extensions/client_credentials.py — nit: the PrivateKeyJWT-side docstring examples in this file still construct providers without issuer=, so a user copying the SDK's own documented code now gets the very UserWarning this PR adds (and a pytest suite with warnings-as-errors fails), which the base branch did not do. Fix: add issuer="https://auth.example.com" (or a one-line note about the warning) to every example in this module that omits it — the PrivateKeyJWTOAuthProvider class docstring Options 1-3 (lines 273-310), static_assertion_provider (174-179), and SignedJWTParameters (208-213); the ClientCredentialsOAuthProvider example already has it. [also at: src/mcp/client/auth/extensions/client_credentials.py:309 - nit: the PrivateKeyJWTOAuthProvider class docstring example constructs the provider without issuer=, so anyone…]

  Extended reasoning...

  This PR makes _checked_issuer(None) emit a UserWarning at construction (src/mcp/client/auth/extensions/client_credentials.py:27-33), and it updated the docs page, the example story, and every in-repo test to pass issuer=. But the module's own public docstrings were only updated in their Args sections: the PrivateKeyJWTOAuthProvider class docstring shows three copy-paste examples (Option 1 ~lines 273-279, Option 2 ~286-292, Option 3 ~304-310), and static_assertion_provider (~174-179) and SignedJWTParameters (~208-213) show two more, all constructing the provider without issuer=. A developer who copies any of these verbatim — the normal way these docstrings are consumed — now sees UserWarning: No issuer given... on every construction, and if their test suite runs with filterwarnings = ["error"] (the convention this repo itself follows) their tests fail outright; on the base branch the same copied code ran silently. The ClientCredentialsOAuthProvider class example (lines 75-82) already includes issuer=, showing the intended pattern; the five…

  Verification: nit — triggered whenever a user copies one of this module's own docstring examples verbatim. Mechanism verified: _checked_issuer() at src/mcp/client/auth/extensions/client_credentials.py:27-33 now emits warnings.warn("No issuer given: ...", stacklevel=3) when issuer is None, and the PR updated docs/client/oauth-clients.md, the example story, and all in-repo tests to pass issuer= —…

I re-reviewed the rework that turned the construction warning into an MCPDeprecationWarning and found no bugs; since omitting issuer= now formally becomes required in 3.0, a maintainer's eye on that compatibility-contract commitment is still worthwhile.

What was reviewed: the _checked_issuer restructure in src/mcp/client/auth/extensions/client_credentials.py is behavior-preserving (None warns and falls through as before; non-None keeps the http(s) check), and stacklevel=3 correctly attributes the warning to the constructor call site, pinned by the new warning.filename == __file__ test.
Checked every in-repo construction site against the warnings-as-errors pytest config: all tests, the docs_src tutorial, and the example story now pass issuer= or use pytest.warns; the only remaining no-issuer caller is the conformance harness script, which runs outside pytest as the description notes.
Also confirmed the warning message in the new test matches the source string exactly, and that MCPDeprecationWarning was already exported from mcp, so no new public-API surface is added.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

Code review found no issues

No high-confidence issues detected in this change.

Code review found no issues

No high-confidence issues detected in this change.