Conversation
d9617a3 to
369b704
Compare
fa2f0ae to
5b6eac8
Compare
Jasonvdb
changed the title
6383e32 to
2462854
Compare
c480f2f to
f5ca889
Compare
pubky.ts now imports uuid for session ids, and deletePubky clears the per-session keychain secrets, so the suite mocks both.
pubky.ts imports the singleton store to drop a borrowed identity whose shared credential disappeared, which pulled the real store into the suite.
Session secrets moved from persisted Redux into the Keychain, so removing the Redux entry no longer clears the sessions Ring created for a borrowed identity. Route every borrowed disconnect through one helper that clears them first.
Publishing a signed pkarr record is an ownership action, so it belongs to the app that owns the key. The batch republish now skips identities sourced from Bitkit, and the Settings republish section is gated on owned identities so a borrowed-only user is never offered it. This also removes the path where a transient shared-credential failure during a maintenance action would drop the borrowed reference.
performAuth republished the selected identity's homeserver record after authorising a third-party app. For a borrowed identity that signs an ownership statement with the source app's key, using a homeserver value cached at connect time that the owner may since have changed.
…gone Deleting an owned identity cleared its homeserver session secrets before the abortable private-key step. A failure there returned with the Redux entry and its session list intact but the matching secrets already destroyed, so those grants could never be revoked. The secrets now go after the private records are confirmed deleted and before the Redux entry that lists them, so every abort leaves the identity intact and still revocable.
Dropping a borrowed reference mutated identity state outside the lifecycle gate every other change takes, so it could interleave with a connect that had already written a session secret to the Keychain but not yet dispatched the session, leaving that grant secret unreachable and unrevocable. The disconnect now runs under the gate and re-checks that the identity is still present and still borrowed before touching anything.
f5ca889 to
09095c4
Compare
|
@codex review |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 254a858d0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 987e504ea9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e8a6fff70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66abc213c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8d672642b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const displayedContent = rootState.routes[displayedContentIndex]; | ||
| const hasMatchingDetail = | ||
| displayedContent?.name === 'PubkyDetail' && getActiveRoutePubky(displayedContent) === pubky; |
There was a problem hiding this comment.
When the disconnected identity's detail screen is below another non-sheet screen, this checks only the currently displayed content route and returns without removing the stale detail. For example, a user can double-tap the logo on PubkyDetail to open Settings; if Bitkit revokes sharing while Settings is active, the foreground refresh disconnects the identity, but going back then reveals the blank detail screen. Fresh evidence in the current tree is the PubkyDetail -> Settings route created by AppHeader.handleDoubleTap, which this displayed-route-only check does not cover. Search all root routes for the matching PubkyDetail, rather than only displayedContent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 04a0a73. removeDisconnectedPubkyDetail now removes matching detail routes anywhere in the root stack, including beneath Settings, while preserving unrelated routes and the active screen. Regression tests cover this, and manual Android/iOS checks confirmed Settings stays open and Back returns Home after source removal.
|
Two independent reviews. needs changing before merge
worth doing, does not block
nits
the reviewers disagree, your call
|
|
@coreyphillips Addressed both blockers and the smaller fixes in five signed commits:
The lifecycle-lock/network refactor is deferred because releasing the gate needs separate race handling. The PR documents the iOS URL-scheme limitation: another app registering Validation: 29 Jest suites / 245 tests, native iOS tests, TypeScript, and changed-file formatting passed. ESLint has no errors and nine unchanged warnings. Manual Android/iOS tests passed for adoption, restart, immediate reoffer/reconnect, stale-sheet closure, and source removal beneath Settings. iOS generated fixtures also passed actual reinstall recovery, private-data preservation, backup entry, deletion, and restart without ghost cards. The checklist is updated. CI is running. |
3 participants
Summary
Relaunch Android Ring as
app.pubkyring2.0 and allow Ring and Bitkit to explicitly use each other's Pubky identities while the source app retains ownership.pubky.sharedKeychain group withWhenUnlockedThisDeviceOnlyprotection.Release order and gates
The legacy Android sunset app v1.17 has rolled out. Bitkit iOS #636 and Android #1084/#1097 are merged. Release Ring before the companion Bitkit PRs.
pubky.sharedfor both App IDs, regenerate profiles, and pass signed two-app physical-device tests.Migration notes and limitations
The app-private Ring identity remains canonical. Shared mirrors are verified before stale mirrors are pruned or private identities are deleted. Borrowed identities are never promoted to owned identities. On iOS, uninstalling the app does not remove Keychain identities.
On iOS,
canOpenURL(bitkit://)is an availability hint, not proof that Bitkit is installed or a security boundary. Another app registering that scheme can prevent uninstall detection while a retained shared record exists. Keychain access remains restricted by the shared access group.The existing lifecycle lock still spans connection network calls. Shortening that lock is deferred because it needs separate race handling.
Validation
Current head
97f83fb2contains mainf14243688. The five follow-up commits are signed.git diff --checkpassed.Manual QA
Tests used disposable zero-balance wallets and staging or generated local identities. The dated rows cover this follow-up; other checked scenarios are previously completed coverage, not claims that everything was rerun. Simulator results do not replace the Apple physical-device gate. The iOS uninstall scenario is subject to the URL-scheme limitation above.
Sep 22, iOS generated-fixture reinstall: both retained private formats restore visible owned cards, preserve exact private bytes and metadata, open backup entry, and delete normally without returning after restart.
Sep 22, both platforms: adopt/reconnect a Bitkit key, immediately reoffer after Disconnect, close a disappeared offer's sheet, and preserve Settings while removing the disconnected detail beneath it.
Both platforms: owned-card body/action, backup entry, and drag behave independently.
Both platforms: adopt the exact Bitkit-owned key in Ring; show borrowed ownership and no Backup action.
Both platforms: restart both apps with the Bitkit source retained; the same borrowed Ring profile restores.
Both platforms: delete/uninstall/reset the Bitkit source; foregrounding Ring disconnects the borrowed profile while preserving owned keys.
Android: reject discovery from a nonempty wrong-certificate source in both directions, with trusted-signer controls.
Both platforms: authorize a borrowed Bitkit identity through Ring and independently receive/decrypt the exact grant.
Android Ring → Bitkit: exact-key adoption among two choices, empty-contact route, restart, ownership preservation, and Ring-uninstall disconnection.
iOS Ring → Bitkit: exact-key selection among four choices, Pay Contacts, restart persistence, and Ring-uninstall disconnection.
iOS upgrade: old private keys remain readable; new keys use sync=false; deletion removes private/shared entries without harming old keys.
Android rebuilt Ring: create, restart, delete, restart empty; Bitkit discovery omits the deleted identity.
Both platforms: published Ring profile displays in Bitkit and explicitly selected contacts import with exact public keys.
Both platforms: an owned encrypted Ring backup decrypts to the exact owned identity while a borrowed identity coexists.
Both platforms: full watch-only approval using a borrowed Ring identity; requester verifies the signed xpub claim and exact two-path grant.
Both platforms: Ring authorizes a borrowed Bitkit identity; requester verifies the exact key and capabilities.
Automated fault injection: shared-removal failure blocks private deletion and retry preserves shared-before-private ordering.
Wipe recovery: failed final shared-store verification removes only proven-keyless owned cards, preserves surviving/borrowed identities, reports the error, and remains retryable.
Source-loss navigation: matching detail and identity-specific sheets close while unrelated and nested routes are preserved.
iOS pre-sharing update: private record, shared mirror, encrypted backup, and ownership survive update and cold restart.
Android legacy migration: v1.17 side-by-side recovery and v1.16 → v1.17 in-place update preserve exact identities.
iOS isolated synthetic wipe: tested legacy/copied-style private keys, sessions, and Ring mirrors are removed; another source's mirror remains unchanged.