Gate on grant_types_supported skips refresh instead of terminating - #58
Merged
Conversation
🦋 Changeset detectedLatest commit: 8b826f4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
germ-mark
force-pushed
the
feat/refresh-token-gate
branch
2 times, most recently
from
July 29, 2026 08:27
fe9d7a4 to
0f47abc
Compare
return nil from the refresh closure is the terminate-session signal - the adopter moves to .expired and logs the user out. A server whose metadata lists grant types without refresh_token (mastodon.social advertises authorization_code and client_credentials) killed the session on the first refresh attempt instead of living until access-token expiry. Throwing preserves the previous state, which is the skip the changeset already describes. Thrown as a dedicated refreshNotSupported so callers can tell "this server can never refresh - plan around access-token expiry" apart from a transient failure; notSupported stays the PAR-endpoint signal. The nil-leniency stands, now with the RFC 8414 deviation reasoned in a comment: absent grant_types_supported strictly defaults to no refresh, but servers routinely omit it while supporting refresh, and one that truly does not answers unsupported_grant_type - which also preserves the session. Tests cover the gated server (throws, zero token requests), omitted metadata (request sent), and advertised support (request sent). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
refreshNotSupported is public API and deserves its own changelog entry, including the exhaustive-switch caveat. And the test session does not mirror the production adopter - production swallows the throw and returns the old token - it exercises the nil-vs-throw contract, so say that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
germ-mark
force-pushed
the
llm/refresh-gate-skip-not-terminate
branch
from
July 29, 2026 08:30
fa45722 to
8b826f4
Compare
Merged
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
1 participant
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.
Suggestion into #52 — targets
feat/refresh-token-gate.The defect, proven against the real adopter:
return nilfrom the refresh closure is the terminate-session signal (AtprotoOAuth:nil→.expired→ logged out), so a server whose metadata lists grant types withoutrefresh_tokenlogged the user out on the first refresh attempt instead of the session living until access-token expiry. Livemastodon.socialadvertises["authorization_code","client_credentials"]— with #47's Mastodon demo in this same stack, this would have bitten immediately. It also inverted #53: that scenario is preserved on #53 alone but was terminated once #52 merged over it.The fix: throw a dedicated
OAuth.Errors.refreshNotSupportedinstead. Throwing preserves the previous state under the adopter contract, which is the "skip" the changeset already describes — the changelog text becomes accurate rather than aspirational. A dedicated case (rather than reusingnotSupported, which after #49 is the PAR signal) lets callers distinguish "this server can never refresh — plan around access-token expiry" from a transient failure.Kept: the
!= falseleniency for omittedgrant_types_supported. Strict RFC 8414 reading says absent defaults to no refresh, but servers routinely omit the field while supporting refresh, and one that truly doesn't answersunsupported_grant_type— which is preserved under #53's classification too. The deviation is now reasoned in a comment rather than implicit.Branch shape:
feat/refresh-token-gate+origin/feat/token-revocation+origin/mainmerged in (both clean), so this builds and tests against the true post-stack state — 0.3.0, the mocks, and #53's classification together. Once #51 merges and #52 retargets tomain, the merge commits' contents drop out of the diff.Test notes
15 tests, 5 suites, all green — including #53's four refresh-error tests unmodified, and three new: gated server throws
refreshNotSupportedwith zero token-endpoint requests recorded, omittedgrant_types_supportedstill refreshes, advertised support refreshes.🤖 Generated with Claude Code