fix(menubar): make a failed disconnect leave a consistent state

disconnect() cleared the usage block before anyone knew whether the delete
had worked, and AppStore then returned early on failure — so a failed
disconnect cleared some state, left the rest, and still posted
subscriptionDisconnected. It also carried a second !isSuccess branch that the
early return had already made unreachable.

Both services now return the delete result and only clear the usage block on
success, so a failure changes nothing at all: the provider stays connected,
Disconnect stays available, and the banner asks for a retry. That matches the
success path's ordering instead of half-applying it.

Errors reaching the generic catches now render localizedDescription rather
than String(describing:), so a Keychain failure shows its message instead of
an enum dump with the raw item name in it.
This commit is contained in:
iamtoruk 2026-08-19 11:43:44 -07:00
parent 0fd8419bfd
commit 0f7bfb3eb2
3 changed files with 32 additions and 29 deletions

View file

@ -1053,7 +1053,7 @@ final class AppStore {
return false
} catch {
guard gen == claudeRefreshGen else { return false }
subscriptionError = sanitizeForUI(String(describing: error))
subscriptionError = sanitizeForUI(error.localizedDescription)
subscriptionLoadState = .failed
return false
}
@ -1064,24 +1064,20 @@ final class AppStore {
/// account or tier) starts clean. capacityEstimates and the snapshot store
/// would otherwise contaminate "Based on last cycle" projections.
func disconnectSubscription() {
ClaudeSubscriptionService.disconnect()
let result = ClaudeSubscriptionService.disconnect()
// Bump the generation token so any in-flight refreshSubscription that
// resumes after this point detects the disconnect and discards its
// result instead of re-populating the cleared state.
claudeRefreshGen &+= 1
if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess {
// Any leftover Keychain item or plaintext must keep Disconnect
// so the user can retry delete.
guard result.isSuccess else {
// Nothing was removed, so nothing is disconnected. Leave the
// connected state exactly as it was the bootstrap flag is still
// set, Disconnect stays available, and the banner says to retry.
subscriptionError = "Could not fully remove the local Claude credential cache. Disconnect again to retry."
NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil)
return
}
subscription = nil
if let result = ClaudeCredentialStore.lastCacheDeleteResult, !result.isSuccess {
subscriptionError = "Could not fully remove the local Claude credential cache."
} else {
subscriptionError = nil
}
subscriptionError = nil
subscriptionLoadState = .notBootstrapped
capacityEstimates = [:]
Task.detached { await SubscriptionSnapshotStore.clearAll() }
@ -1102,7 +1098,7 @@ final class AppStore {
} catch let err as CodexSubscriptionService.FetchError {
applyCodexFetchError(err)
} catch {
codexError = sanitizeForUI(String(describing: error))
codexError = sanitizeForUI(error.localizedDescription)
codexLoadState = .failed
}
}
@ -1135,26 +1131,23 @@ final class AppStore {
return false
} catch {
guard gen == codexRefreshGen else { return false }
codexError = sanitizeForUI(String(describing: error))
codexError = sanitizeForUI(error.localizedDescription)
codexLoadState = .failed
return false
}
}
func disconnectCodex() {
CodexSubscriptionService.disconnect()
let result = CodexSubscriptionService.disconnect()
codexRefreshGen &+= 1
if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess {
guard result.isSuccess else {
// Nothing removed means nothing disconnected; keep state intact so
// Disconnect stays available for a retry.
codexError = "Could not fully remove the local Codex credential cache. Disconnect again to retry."
NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil)
return
}
codexUsage = nil
if let result = CodexCredentialStore.lastCacheDeleteResult, !result.isSuccess {
codexError = "Could not fully remove the local Codex credential cache."
} else {
codexError = nil
}
codexError = nil
codexLoadState = .notBootstrapped
NotificationCenter.default.post(name: .codeBurnSubscriptionDisconnected, object: nil)
}
@ -1196,7 +1189,7 @@ final class AppStore {
applyKimiFetchError(err)
} catch {
guard gen == kimiRefreshGen else { return }
kimiError = sanitizeForUI(String(describing: error))
kimiError = sanitizeForUI(error.localizedDescription)
kimiLoadState = .failed
}
}
@ -1230,7 +1223,7 @@ final class AppStore {
return false
} catch {
guard gen == kimiRefreshGen else { return false }
kimiError = sanitizeForUI(String(describing: error))
kimiError = sanitizeForUI(error.localizedDescription)
kimiLoadState = .failed
return false
}

View file

@ -99,9 +99,14 @@ enum ClaudeSubscriptionService {
}
/// Reset everything used on user-initiated disconnect.
static func disconnect() {
_ = ClaudeCredentialStore.resetBootstrap()
clearUsageBlock()
/// Returns the delete outcome so callers only tear down UI state once the
/// credential material is actually gone. A failed delete leaves the usage
/// block intact too, so a retry starts from the same state.
@discardableResult
static func disconnect() -> ClaudeCredentialStore.CacheDeleteResult {
let result = ClaudeCredentialStore.resetBootstrap()
if result.isSuccess { clearUsageBlock() }
return result
}
// MARK: - Internal

View file

@ -78,9 +78,14 @@ enum CodexSubscriptionService {
}
}
static func disconnect() {
_ = CodexCredentialStore.resetBootstrap()
clearUsageBlock()
/// Returns the delete outcome so callers only tear down UI state once the
/// credential material is actually gone. A failed delete leaves the usage
/// block intact too, so a retry starts from the same state.
@discardableResult
static func disconnect() -> CodexCredentialStore.CacheDeleteResult {
let result = CodexCredentialStore.resetBootstrap()
if result.isSuccess { clearUsageBlock() }
return result
}
private static func fetchWithToken(_ token: String, allowOne401Recovery: Bool) async throws -> CodexUsage {