[PR #32] [MERGED] feat(auth): ComfyTokenStore protocol + ComfyAuth.restoreClient / signOut #34

New issue

Closed

opened 2026-10-26 22:34:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:34:11 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/32
Author: @mattmillerai
Created: 7/6/2026
Status: ✅ Merged
Merged: 7/7/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-2450-token-store-restore-client

📝 Commits (4)

c2ae295 feat(auth): parameterize OAuth client config + add callback state-verification helper
c1ca10c fix(auth): thread OAuthClientConfig through refresh + tighten callback checks
5a005b2 feat(auth): add ComfyTokenStore + ComfyAuth.restoreClient/signOut
337be8a fix(auth): normalize store read errors + redact token secrets

📊 Changes

15 files changed (+991 additions, -69 deletions)

View changed files

📝 Sources/ComfySwiftSDK/Internal/Auth/OAuthConfiguration.swift (+4 -12)
📝 Sources/ComfySwiftSDK/Internal/Auth/OAuthExchanger.swift (+7 -3)
📝 Sources/ComfySwiftSDK/Internal/Auth/OAuthTokenRefreshExecutor.swift (+9 -2)
📝 Sources/ComfySwiftSDK/Internal/PollingFallback.swift (+2 -2)
📝 Sources/ComfySwiftSDK/Internal/SDKLog.swift (+2 -0)
📝 Sources/ComfySwiftSDK/Internal/Transport.swift (+9 -2)
➕ Sources/ComfySwiftSDK/Public/ComfyAuth.swift (+246 -0)
📝 Sources/ComfySwiftSDK/Public/ComfyCloudClient.swift (+42 -17)
📝 Sources/ComfySwiftSDK/Public/ComfyError.swift (+11 -0)
📝 Sources/ComfySwiftSDK/Public/OAuthAuthorizationRequest.swift (+43 -1)
➕ Sources/ComfySwiftSDK/Public/OAuthClientConfig.swift (+78 -0)
➕ Tests/ComfySwiftSDKTests/ComfyAuthTests.swift (+323 -0)
➕ Tests/ComfySwiftSDKTests/OAuthClientConfigTests.swift (+176 -0)
📝 Tests/ComfySwiftSDKTests/OAuthConfigurationTests.swift (+2 -29)
📝 Tests/ComfySwiftSDKTests/OAuthExchangeTests.swift (+37 -1)

📄 Description

ELI-5

Signing an app into Comfy Cloud means holding three things — an access token, a refresh token, and when the access token expires — and re-wiring four little callbacks so the SDK can quietly refresh them. Every app was copy-pasting that same four-callback boilerplate over its own keychain. This PR moves it into the SDK: an app just hands the SDK a tiny "here's where I keep my tokens" object, and gets back a ready, self-refreshing client. Signing out becomes one call.

What & why

Replaces the copy-paste four-closure .oauthRefreshable wiring with a single injectable token-store protocol. Generalizes the pattern that Comfy-iOS's HomeViewModel.resolveCredential() hand-builds today.

New public surface (all Foundation-only, DocC'd):

ComfyStoredTokens — a Sendable storage view of a token pair with an absolute expiresAt (vs. the wire shape's relative expiresIn), so a token reloaded on a later launch still knows when it lapses.
ComfyTokenStore — a Sendable load / save / clear persistence protocol. Implement it over Keychain, an encrypted file, or (in tests) an in-memory box.
ComfyAuth.restoreClient(store:config:) — returns nil when the store holds no usable session (nil, or an empty access token); otherwise returns a ComfyCloudClient in .oauthRefreshable mode whose four closures adapt the store:
- token provider → current access token, or .authInvalid if missing/empty
- refresh provider → current refresh token, or .authExpired if missing/empty
- token store → persists each refreshed pair; a save failure surfaces as .unknown(underlying:), never .authInvalid (matches the existing OAuthTokenStore contract, so an infra write error isn't misreported as an invalid credential)
- expiry provider → when the access token lapses, for proactive refresh
ComfyAuth.signOut(store:) — clears the store.

Plus an internal ComfyStoredTokens(response:now:) mapper (OAuthTokenResponse.expiresIn + a reference instant → absolute expiresAt), so the expiry arithmetic lives in one tested place.

Judgment call — sync `expiryProvider` over an async store

The SDK's OAuthExpiryProvider is synchronous (() throws -> Date?), but ComfyTokenStore.load() is async, so the expiry can't be read from the store on the proactive-refresh check. I bridge it with a small lock-guarded in-memory expiry cache: seeded from the load() that gated restoreClient, and updated by the token-store closure on every successful refresh-save. The alternative — returning nil — makes the SDK treat the token as always-expired and refresh before every request (a refresh storm). A stale cache only ever mis-times the proactive refresh; the 401-triggered refresh path remains the backstop, so it's safe either way.

Stacked on the OAuth-client-config PR

This depends on the parameterized OAuthClientConfig / .comfyIOS from #31 and is opened with #31 as its base, so the diff here is just the token-store glue. GitHub will retarget it to main once #31 merges.

Testing

New ComfyAuthTests (15 tests): an in-memory ComfyTokenStore fake; restoreClient → nil when empty / nil on empty access token / rethrows a load failure / builds a client when populated; each of the four adapter closures (including the .unknown-not-.authInvalid save-failure contract); the expiry cache seeds + updates after a refresh-save; the expiresIn→expiresAt mapping (incl. zero); signOut clears the store.
swift build, full swift test (198 tests, all pass), and ImportBoundaryTests green. Foundation-only (no Security).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-swift-sdk/pull/32 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/6/2026 **Status:** ✅ Merged **Merged:** 7/7/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-2450-token-store-restore-client` --- ### 📝 Commits (4) - [`c2ae295`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/c2ae295562719280e164f08d5acb25e94c8e564c) feat(auth): parameterize OAuth client config + add callback state-verification helper - [`c1ca10c`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/c1ca10cff689e3acdc9073a894ec4180f91a3829) fix(auth): thread OAuthClientConfig through refresh + tighten callback checks - [`5a005b2`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/5a005b2dafc16ec2d34d57e538922c70dee9e009) feat(auth): add ComfyTokenStore + ComfyAuth.restoreClient/signOut - [`337be8a`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/337be8a09f42faadf97b07ea0e1f5ccc4d153cdf) fix(auth): normalize store read errors + redact token secrets ### 📊 Changes **15 files changed** (+991 additions, -69 deletions) <details> <summary>View changed files</summary> 📝 `Sources/ComfySwiftSDK/Internal/Auth/OAuthConfiguration.swift` (+4 -12) 📝 `Sources/ComfySwiftSDK/Internal/Auth/OAuthExchanger.swift` (+7 -3) 📝 `Sources/ComfySwiftSDK/Internal/Auth/OAuthTokenRefreshExecutor.swift` (+9 -2) 📝 `Sources/ComfySwiftSDK/Internal/PollingFallback.swift` (+2 -2) 📝 `Sources/ComfySwiftSDK/Internal/SDKLog.swift` (+2 -0) 📝 `Sources/ComfySwiftSDK/Internal/Transport.swift` (+9 -2) ➕ `Sources/ComfySwiftSDK/Public/ComfyAuth.swift` (+246 -0) 📝 `Sources/ComfySwiftSDK/Public/ComfyCloudClient.swift` (+42 -17) 📝 `Sources/ComfySwiftSDK/Public/ComfyError.swift` (+11 -0) 📝 `Sources/ComfySwiftSDK/Public/OAuthAuthorizationRequest.swift` (+43 -1) ➕ `Sources/ComfySwiftSDK/Public/OAuthClientConfig.swift` (+78 -0) ➕ `Tests/ComfySwiftSDKTests/ComfyAuthTests.swift` (+323 -0) ➕ `Tests/ComfySwiftSDKTests/OAuthClientConfigTests.swift` (+176 -0) 📝 `Tests/ComfySwiftSDKTests/OAuthConfigurationTests.swift` (+2 -29) 📝 `Tests/ComfySwiftSDKTests/OAuthExchangeTests.swift` (+37 -1) </details> ### 📄 Description ## ELI-5 Signing an app into Comfy Cloud means holding three things — an access token, a refresh token, and when the access token expires — and re-wiring four little callbacks so the SDK can quietly refresh them. Every app was copy-pasting that same four-callback boilerplate over its own keychain. This PR moves it into the SDK: an app just hands the SDK a tiny "here's where I keep my tokens" object, and gets back a ready, self-refreshing client. Signing out becomes one call. ## What & why Replaces the copy-paste four-closure `.oauthRefreshable` wiring with a single injectable token-store protocol. Generalizes the pattern that Comfy-iOS's `HomeViewModel.resolveCredential()` hand-builds today. New public surface (all Foundation-only, DocC'd): - **`ComfyStoredTokens`** — a `Sendable` storage view of a token pair with an **absolute** `expiresAt` (vs. the wire shape's relative `expiresIn`), so a token reloaded on a later launch still knows when it lapses. - **`ComfyTokenStore`** — a `Sendable` `load` / `save` / `clear` persistence protocol. Implement it over Keychain, an encrypted file, or (in tests) an in-memory box. - **`ComfyAuth.restoreClient(store:config:)`** — returns `nil` when the store holds no usable session (nil, or an empty access token); otherwise returns a `ComfyCloudClient` in `.oauthRefreshable` mode whose four closures adapt the store: - token provider → current access token, or `.authInvalid` if missing/empty - refresh provider → current refresh token, or `.authExpired` if missing/empty - token store → persists each refreshed pair; **a save failure surfaces as `.unknown(underlying:)`, never `.authInvalid`** (matches the existing `OAuthTokenStore` contract, so an infra write error isn't misreported as an invalid credential) - expiry provider → when the access token lapses, for proactive refresh - **`ComfyAuth.signOut(store:)`** — clears the store. Plus an internal `ComfyStoredTokens(response:now:)` mapper (`OAuthTokenResponse.expiresIn` + a reference instant → absolute `expiresAt`), so the expiry arithmetic lives in one tested place. ## Judgment call — sync `expiryProvider` over an async store The SDK's `OAuthExpiryProvider` is **synchronous** (`() throws -> Date?`), but `ComfyTokenStore.load()` is **async**, so the expiry can't be read from the store on the proactive-refresh check. I bridge it with a small lock-guarded in-memory expiry cache: seeded from the `load()` that gated `restoreClient`, and updated by the token-store closure on every successful refresh-save. The alternative — returning `nil` — makes the SDK treat the token as always-expired and refresh before *every* request (a refresh storm). A stale cache only ever mis-times the *proactive* refresh; the 401-triggered refresh path remains the backstop, so it's safe either way. ## Stacked on the OAuth-client-config PR This depends on the parameterized `OAuthClientConfig` / `.comfyIOS` from #31 and is opened **with #31 as its base**, so the diff here is just the token-store glue. GitHub will retarget it to `main` once #31 merges. ## Testing - New `ComfyAuthTests` (15 tests): an in-memory `ComfyTokenStore` fake; `restoreClient` → nil when empty / nil on empty access token / rethrows a load failure / builds a client when populated; each of the four adapter closures (including the `.unknown`-not-`.authInvalid` save-failure contract); the expiry cache seeds + updates after a refresh-save; the `expiresIn`→`expiresAt` mapping (incl. zero); `signOut` clears the store. - `swift build`, full `swift test` (**198 tests, all pass**), and `ImportBoundaryTests` green. Foundation-only (no `Security`). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:34:11 +00:00