Skip to content

Conversation

@hildjj
Copy link
Contributor

@hildjj hildjj commented Oct 23, 2024

@Mikescops Mikescops requested a review from Copilot July 11, 2025 14:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds an optional account parameter to all keychain operations, updates native Swift bindings, tests, and documentation to support multi-account scenarios, and bumps dependencies.

  • Extend setPassword, getPassword, and deletePassword APIs (TypeScript and Swift) to accept an account argument.
  • Update tests and README examples to demonstrate the new account parameter.
  • Upgrade koffi and various dev dependencies to their latest versions.

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/index.ts Add and verify set/get/delete flows with the account parameter
src/index.ts Extend function signatures and native calls to include account
package.json Bump koffi and development dependency versions
Sources/KeychainLibrary.swift Update Swift keychain functions to accept and use account
README.md Update usage examples to include the account parameter
Comments suppressed due to low confidence (3)

tests/index.ts:27

  • Consider adding a test that calls getPassword with requireBiometry: true and an account to cover the biometric-protected retrieval flow when using the new account parameter.
    const getAccount = await keychain.getPassword({

README.md:37

  • The account parameter isn’t documented under the Available methods section; update these method listings to include account for setPassword, getPassword, and deletePassword.
## Available methods

tests/index.ts:27

  • [nitpick] The variable getAccount holds the retrieved secret; consider renaming it to retrievedPassword or secret for clearer intent.
    const getAccount = await keychain.getPassword({

@Mikescops
Copy link
Owner

Thanks a lot for this PR, I'm sorry I didn't check at all until now.. but here we are.

@Mikescops Mikescops merged commit d650f76 into Mikescops:main Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add "account" to get/set/delete

2 participants