Split out of the review panel on #44, which flagged both as High/Medium but deferred them: #44 is a no-behavior-change extraction of a shared token-endpoint helper, and both gaps pre-date it (OAuthExchanger and OAuthTokenRefreshExecutor on main each already decoded these as required). Fixing them properly is a breaking public API change, so it gets its own ticket.
The gap
OAuthTokenEndpoint.TokenDTO decodes both fields as required:
let refreshToken: String
let expiresIn: Int
Neither is required by RFC 6749:
refresh_token (§6) — on a refresh grant the server may omit it, meaning "keep using the one you have." Many providers do not rotate on every refresh. When absent, JSONDecoder throws keyNotFound, which OAuthTokenEndpoint.post wraps as ComfyError.unknown. The 401 retry path in Transport only recovers from .authInvalid/.authExpired, so .unknown escapes: a spec-compliant refresh fails and forces full re-authentication.expires_in (§5.1) — only RECOMMENDED, not required. An omitting server throws during decode and surfaces as ComfyError.unknown rather than a usable token.
Why it is not a one-line fix
OAuthTokenResponse is a public type (Sources/ComfySwiftSDK/Public/OAuthTokenResponse.swift):
public let refreshToken: String
public let expiresIn: Int
So a correct fix has to decide, deliberately:
- Whether the public fields become optional (source-breaking for SDK consumers) or keep their non-optional shape and get filled in internally.
- For
refresh_token: thread a "reuse the prior refresh token when absent" fallback through the refresh caller and whatever persists the token. The DTO alone cannot do this — only the caller knows the previous token.
- For
expires_in: pick a default, and check how it interacts with proactive-refresh scheduling.
Option worth considering: keep OAuthTokenResponse non-optional (no consumer break), make the DTO fields optional, and have OAuthTokenEndpoint.post take the prior refresh token + a default lifetime to fill gaps. That contains the change to internals.
Acceptance
- A refresh response omitting
refresh_token succeeds and retains the previous refresh token.
- A token response omitting
expires_in decodes to a usable token with a documented default.
- Neither case surfaces as
ComfyError.unknown.
- Regression tests for both, plus an explicit note in the changelog if the public API shape changes.
Split out of the review panel on #44, which flagged both as High/Medium but deferred them: #44 is a no-behavior-change extraction of a shared token-endpoint helper, and both gaps pre-date it (
OAuthExchangerandOAuthTokenRefreshExecutoronmaineach already decoded these as required). Fixing them properly is a breaking public API change, so it gets its own ticket.The gap
OAuthTokenEndpoint.TokenDTOdecodes both fields as required:Neither is required by RFC 6749:
refresh_token(§6) — on a refresh grant the server may omit it, meaning "keep using the one you have." Many providers do not rotate on every refresh. When absent,JSONDecoderthrowskeyNotFound, whichOAuthTokenEndpoint.postwraps asComfyError.unknown. The 401 retry path inTransportonly recovers from.authInvalid/.authExpired, so.unknownescapes: a spec-compliant refresh fails and forces full re-authentication.expires_in(§5.1) — only RECOMMENDED, not required. An omitting server throws during decode and surfaces asComfyError.unknownrather than a usable token.Why it is not a one-line fix
OAuthTokenResponseis a public type (Sources/ComfySwiftSDK/Public/OAuthTokenResponse.swift):So a correct fix has to decide, deliberately:
refresh_token: thread a "reuse the prior refresh token when absent" fallback through the refresh caller and whatever persists the token. The DTO alone cannot do this — only the caller knows the previous token.expires_in: pick a default, and check how it interacts with proactive-refresh scheduling.Option worth considering: keep
OAuthTokenResponsenon-optional (no consumer break), make the DTO fields optional, and haveOAuthTokenEndpoint.posttake the prior refresh token + a default lifetime to fill gaps. That contains the change to internals.Acceptance
refresh_tokensucceeds and retains the previous refresh token.expires_indecodes to a usable token with a documented default.ComfyError.unknown.