gemini-cli - 💡(How to fix) Fix deleteCredentials throws when keychain entry doesn't exist, cascading clearCredentials errors [1 comments, 1 participants]

KeychainTokenStorage.deleteCredentials() throws Error: No credentials found for <serverName> when the underlying keychainService.deletePassword() returns false (entry not present). Make deleteCredentials idempotent — standard delete semantics. A deletePassword returning false (not-found) should be a no-op, not an error. Same semantics as rm -f, Python dict.pop(k, None), etc. // Idempotent: deleting credentials that don't exist is not an error.

TL;DR

Make deleteCredentials idempotent by not throwing an error when keychainService.deletePassword() returns false, indicating the credential does not exist.

Guidance

• Review the proposed code change to ensure it aligns with the desired idempotent behavior of deleteCredentials.
• Update the test suite to include scenarios where some accounts are missing to verify clearAll() succeeds as expected.
• Consider the impact of this change on other parts of the system that may rely on the previous behavior of deleteCredentials.
• Evaluate the necessity of adding error handling for cases where keychainService.deletePassword() fails for reasons other than the credential not existing.

Example

The proposed code change is already provided in the issue:

async deleteCredentials(serverName: string): Promise<void> {
  const sanitizedName = this.sanitizeServerName(serverName);
  // Idempotent: deleting credentials that don't exist is not an error.
  await this.keychainService.deletePassword(sanitizedName);
}

This change directly addresses the issue by removing the error throw when the credential is not found.

Notes

This solution assumes that making deleteCredentials idempotent is the correct approach and does not introduce unintended side effects. It's also important to ensure that the test update covers all relevant scenarios to prevent regressions.

Recommendation

Apply the proposed workaround by making deleteCredentials idempotent, as it directly addresses the issue and aligns with standard delete semantics, allowing for a more robust and user-friendly authentication flow.