Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Test Failure Analysis

Note: Updated to reflect the latest CI run (2026-03-29, workflow run #23705994763).

Summary: The static_analysis job fails because the ty type checker reports a type error in src/fastmcp/server/auth/providers/clerk.py at line 154 — the auth argument to httpx.AsyncClient.post() is typed as None | tuple[...], but httpx does not accept None for that parameter.

Root Cause: In ClerkTokenVerifier.verify_token(), introspect_auth is initialized to None and conditionally assigned a tuple:

introspect_auth = None                          # line 144 — type: None
if self._client_id and self._client_secret:
    introspect_auth = (self._client_id, self._client_secret)   # type: tuple[str, str]
elif self._client_id:
    introspect_data_payload["client_id"] = self._client_id

introspect_response = await client.post(
    self._introspection_url,
    data=introspect_data_payload,
    headers={"User-Agent": "FastMCP-Clerk-OAuth"},
    auth=introspect_auth,   # line 154 — ty error: None is not a valid AuthType
)

httpx.AsyncClient.post() declares auth: AuthTypes | UseClientDefault = USE_CLIENT_DEFAULT. The AuthTypes union does not include None, so passing introspect_auth when it is None is a type error. The fix is to either omit the auth keyword argument when there is no credential, or pass httpx.USE_CLIENT_DEFAULT as the default.

Suggested Solution: In src/fastmcp/server/auth/providers/clerk.py, change the client.post() call to only pass auth when a credential has been set:

# Replace lines 150-155 with:
post_kwargs: dict = {
    "data": introspect_data_payload,
    "headers": {"User-Agent": "FastMCP-Clerk-OAuth"},
}
if introspect_auth is not None:
    post_kwargs["auth"] = introspect_auth

introspect_response = await client.post(
    self._introspection_url,
    **post_kwargs,
)

Alternatively, annotate introspect_auth with its full type and use a conditional branch:

if introspect_auth is not None:
    introspect_response = await client.post(
        self._introspection_url,
        data=introspect_data_payload,
        headers={"User-Agent": "FastMCP-Clerk-OAuth"},
        auth=introspect_auth,
    )
else:
    introspect_response = await client.post(
        self._introspection_url,
        data=introspect_data_payload,
        headers={"User-Agent": "FastMCP-Clerk-OAuth"},
    )

error[invalid-argument-type]: Argument to bound method `post` is incorrect
  --> src/fastmcp/server/auth/providers/clerk.py:154:25
    |
152 |                         data=introspect_data_payload,
153 |                         headers={"User-Agent": "FastMCP-Clerk-OAuth"},
154 |                         auth=introspect_auth,
    |                         ^^^^^^^^^^^^^^^^^^^^ Expected `tuple[str | bytes, str | bytes] | ((Request, /) -> Request) | Auth | UseClientDefault`, found `None | tuple[...]`
    |
info: Element `None` of this union is not assignable to `tuple[str | bytes, str | bytes] | ((Request, /) -> Request) | Auth | UseClientDefault`

Hey @jlowin — could you take a look at this?

The failing test appears to be a pre-existing issue on main, unrelated to the Clerk OAuth changes in this PR.

The marvin-context-protocol bot analysis highlights that get_http_request() does not restore headers when called directly inside a background task (without using CurrentRequest() / CurrentHeaders()), which causes the failure.

@jlowin Thanks for the thoughtful review — really appreciate the detailed feedback.

On the OIDCProxy vs OAuthProxy point:
You’re absolutely right that Clerk is an OIDC provider and that, in theory, using OIDCProxy would simplify things by leveraging JWKS discovery and local JWT verification.

That said, our current approach is intentionally aligned with the existing Google provider pattern, where we fetch user info during token verification. The main reason is that, in practice, we almost always need enriched user attributes (especially email, email_verified, etc.) immediately as part of the auth context.

With OIDCProxy, while we’d get validated JWT claims locally, we’d still need to make a follow-up request to /userinfo to retrieve those fields reliably. So the trade-off becomes:

• OIDCProxy → 0 calls for verification + 1 call for user info
• Current approach → 1–2 calls but returns fully enriched claims in a single step

We chose the latter to keep the auth pipeline simpler for downstream consumers and consistent with existing providers.

On audience validation:
Agreed — this was a gap. The latest update enforces strict audience checking when client_id is configured, ensuring tokens are only accepted if issued for the correct application.

On scope validation fallback:
Also agreed — the previous behavior was too permissive. This is now updated to fail closed:

• If required_scopes is configured but scopes cannot be verified → reject the token
• Scope checks are only skipped when not configured at all

Summary

• We’re aware that OIDCProxy could reduce verification overhead and simplify parts of the implementation.
• However, since user profile data is required in most flows, we’d still need an additional request — so we opted for a design that returns fully enriched claims upfront, consistent with the Google provider.
• The key security concerns (audience + scope validation) are now addressed with strict checks.

@jlowin please take a look — I’ve fixed the oauth/token_info issue using Basic Auth.

@jlowin Thanks — updated.

• Switched to introspection-first (security checks before userinfo)
• Enforced strict active handling (missing ⇒ reject per RFC 7662)
• Made userinfo after introspection (enrichment only)
• Adjusted tests accordingly