fix(mcp): authenticated queries never worked — and the auth knob shouldn't exist#89
Merged
Conversation
query(authenticated=True) has never worked: client.me is the profile view (no access_jwt attribute), so every authenticated query raised AuthenticationRequired *after* a successful login — masquerading as 'no credentials provided'. surfaced by phi's trace audit: 22 identical failures on a ~4h schedule for a month, 100% failure rate. the regression test had encoded the bug — its fake client put the jwt on .me. the fake now matches the real SDK shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
the `authenticated` kwarg was a redundant degree of freedom: the allowlist already fully determines which NSIDs need the caller's session, and both entries are auth-only endpoints. the flag added a knob for callers (LLMs) to guess at, with a misleading failure mode when guessed wrong. now allowlisted NSIDs authenticate automatically and everything else is always public — no caller-supplied knob can widen the boundary. the kwarg is kept, ignored, so existing callers don't fail validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
two commits:
query(authenticated=True)has failed 100% of the time since it shipped (JWT read fromclient.me, which has noaccess_jwt— every call raisedAuthenticationRequiredafter a successful login, blaming missing credentials), and the fix exposed that theauthenticatedkwarg itself was a smell: the allowlist already fully determines which NSIDs need the session, so auth is now automatic and the flag is deprecated-and-ignored.commit 1 — the bug (d5a7592)
getattr(client.me, "access_jwt", None)returning None; the JWT lives onclient._session(aSessiondataclass).me— the fake now matches the real SDK shapecommit 2 — the smell (8ea37aa)
authenticatedflag was a redundant degree of freedom: the server already fully determines auth need from the NSID (the allowlist), and both allowlisted endpoints are auth-onlyauthenticated=Truepassed)🤖 Generated with Claude Code