fix(auth): clear stale tokens after refresh failure to prevent sign-in loop

[Copilot]

After a refresh token request returns 400, logout() → clearAll() can fail partially (e.g., KeychainTokenStorage.clearAll() short-circuits on first delete error), leaving stale tokens in storage. Subsequent checkExistingAuth() calls find the stale refresh token, retry the failed refresh, and loop — user is stuck on the sign-in page.

Changes

• logout() fallback cleanup: When clearAll() throws, individually attempt deleteAccessToken() and deleteRefreshToken() so a partial Keychain failure doesn't leave tokens behind
• checkExistingAuth() catch block: Clear tokens explicitly after a failed refresh, breaking the retry loop regardless of whether handleTokenRefreshFailure() succeeded internally
• checkExistingAuth() missing timer start: Added startTokenRefreshTimer() after successful token refresh (was present in the valid-access-token path but missing in the refresh path)

Tests

Added OAuthManagerTokenCleanupTests covering:

• logout() clears tokens even when clearAll() throws (using a ClearAllFailingTokenStorage mock)
• checkExistingAuth() clears stale tokens after refresh 400 (via MockURLProtocol)
• No retry loop: second checkExistingAuth() call makes zero network requests
• Full cycle: refresh failure → cleanup → new tokens → successful re-authentication

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull request overview

Addresses an auth “sign-in loop” where stale refresh tokens can persist after a token refresh failure, causing repeated refresh attempts on subsequent session checks.

Changes:

• Add a logout() fallback path to delete tokens individually when clearAll() throws.
• Ensure checkExistingAuth() clears tokens after a failed refresh attempt to avoid retries.
• Start the token refresh timer after successfully refreshing from an existing session.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
Sources/RxAuthSwift/OAuthManager.swift	Adds fallback token cleanup on logout, clears tokens on refresh failure, and starts refresh timer after refresh-path restoration.
Tests/RxAuthSwiftTests/OAuthManagerTokenCleanupTests.swift	Adds test coverage for logout cleanup fallback and for breaking the refresh-failure retry loop.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

MockURLProtocol.requestHandler is a global static that isn't reset between tests. Even though you unregister the protocol class, leaving the handler set can create hidden coupling/flakiness if future tests register the protocol without setting a handler, or if tests run in parallel. Consider clearing requestHandler (e.g., defer { MockURLProtocol.requestHandler = nil }) or wrapping register/unregister + handler setup in a helper to ensure isolation.

[Copilot]

In the refresh-token path, the catch block uses try? tokenStorage.clearAll(). Since KeychainTokenStorage.clearAll() can throw before deleting the refresh token (it short-circuits on the first delete error), ignoring the error here can still leave a stale refresh token behind and allow checkExistingAuth() to retry/loop. Consider reusing the same best-effort cleanup as logout() (attempt individual deletions when clearAll() fails), or call await logout() here to guarantee refresh-token removal.

Suggested change

	// Ensure stale tokens are fully cleared so that subsequent
	// calls to checkExistingAuth() don't retry a failed refresh.
	try? tokenStorage.clearAll()
	authState = .unauthenticated
	// Use the same best-effort cleanup as logout() to ensure
	// the refresh token is removed and avoid repeated retries.
	await logout()

[Copilot]

logout()'s fallback cleanup deletes the access/refresh tokens, but it never removes the stored expiration value (KeychainTokenStorage stores it under a separate expires_at key). If clearAll() failed after partially deleting tokens, this can leave a stale expiresAt entry in Keychain. A low-impact fix is to retry clearAll() after the individual deletions (so expires_at gets cleaned up when possible), or extend the protocol with a dedicated expiresAt deletion method.

Suggested change

	try? tokenStorage.deleteRefreshToken()
	try? tokenStorage.deleteRefreshToken()
	// Retry clearAll() to clean up any remaining metadata keys (e.g., expires_at).
	try? tokenStorage.clearAll()