fix(connections): cancel and explain foreign-app keychain prompts#1137
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1134
Problem
When importing connections from TablePlus / Sequel Ace with passwords:
This is by design at the OS level (per-item ACL is intentional to prevent silent cross-app credential siphoning), so the fix is at the UX layer: explain it, give the user a real Cancel, and treat the first denied prompt as "user wants to stop reading credentials".
Changes
ForeignKeychainReader.readPasswordnow returnsKeychainReadResult { found / notFound / cancelled }instead ofString?. Cancellation is no longer indistinguishable from "no value".TablePlusImporterandSequelAceImportercarry anabortFlagthrough their credential reads. On the first.cancelledresult, no further keychain reads happen for the remaining connections.Task.checkCancellation()between connections so an in-app Cancel button propagates.ForeignAppImportResultcarries a newcredentialsAborted: Boolflag (defaulted tofalseso DBeaver, which doesn't touch keychain, is unchanged).ImportFromAppSheetshows a pre-importNSAlertexplaining the per-item prompt behavior and how Always Allow works, only whenInclude passwordsis on. The loading screen has a visible Cancel button that callsTask.cancel().ImportFromAppPreviewStepshows an orange banner above the list when credentials were aborted, telling the user to enter remaining passwords manually after import.Test plan