Repository navigation
fix: stop deletion after incomplete cleanup - #1124
ben-kaufman wants to merge 5 commits into
Conversation
|
Triage: 3.1.0, with its Android twin synonymdev/bitkit-android#1690. |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. The Edit Profile deletion path now uses the throwing private endpoint withdrawal and the throwing deleteAllContacts(), a failed contact read is no longer treated as an empty list, and the best-effort wrappers on this path are gone. Reviewed 76e0f80, full tier, stacked on #1122 (fix/profile-delete-session-recovery), so the delta against that base is what I read.
What I checked, and 5 candidates I ruled out
Read in full: ContactsManager.deleteAllContacts and isMissingContactsDataError, PubkyProfileManager.deleteProfile(for:cleanPrivatePaykitEndpoints:) and both removePrivatePaykitEndpoints overloads, EditProfileView.deleteProfile / performDeleteProfile, PaykitSdkService.contactRecords / removeContacts, and remove_contacts_and_block_peers / contact_records in paykit-rs.
Call sites traced: deleteAllContacts (only EditProfileView), deleteAllContactsBestEffort (no callers left), removePrivatePaykitEndpointsBestEffort (now only AppReset.wipe, out of this PR's scope), pubkyProfile.deleteProfile (only EditProfileView, with cleanPrivatePaykitEndpoints: false).
CI: SwiftFormat green; Run Tests still running when this was posted.
Ruled out
- A missing-storage read now blocks deletion:
contactRecordsis a local SDK storage transaction, so a 404-looking message there is not evidence of an empty list; the list loader keeps its ownmissingDataIsEmptypath, so only deletion changed. deleteProfile(cleanPrivatePaykitEndpoints: true)now throws where it used to continue: no caller passestrue(the default) today, so no flow changes behaviour through it.- Session switch between cleanup steps:
requireCurrentSessionis still checked after each step in the extractedperformDeleteProfile, and the view's catch ignores errors when the public key changed. - Contacts the SDK retains by design (active recurring request, peer lease, published marker) block deletion on every retry: the stated intent of the PR and its journey; the existing
PreservesContactsNotRemovedByTheBulkOperationtest pins it. - Parity with bitkit-android#1690: both stop on endpoint failure before touching contacts, both propagate the contact read error, both keep unacknowledged contacts and overrides; iOS additionally clears local-only overrides on full success, which Android gets from the store reset in
signOut.
Not built here: iOS does not build on this reviewer's Linux host, so this is reasoned from code and CI.
Merge confidence: 4/5, no findings, but the change is reasoned from code rather than built here, and the unit test job had not finished.
Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no findings.
Checked and clean:
- When cleanup is incomplete, the user gets the existing Retry / Sign out / Cancel alert. Retry is idempotent after a late
signOutfailure, because an absent profile reads asNone. - No path locks the user out of deletion or sign-in for good. Private endpoints are now withdrawn before the profile delete, and the cleanup flag is cleared on any successful withdrawal (
PubkyProfileManager.swift:1498), so the user's private lists come back right after Cancel. profile-cleanup-failure.xml's fault point is right: contact removal goes through the shared-state storage adapter (PubkyShared,put_locked), so a homeserver proxy can fail it.- With #1122 removing the automatic replay, each stop is final until a manual Retry.
- Twin synonymdev/bitkit-android#1690 keeps its cleanup flag set after a stop; that's raised there.
Device gate (partial): a062618 — not run yet. profile-cleanup-failure.xml needs a homeserver fault proxy. The partial-removal stop can be driven without one, using a contact with an active recurring payment request on two devices. Device results will follow in a separate review.
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for a062618: iPhone 17 and iPhone SE simulators, iOS 26.5; bitkit-core 0.8.0-rc11 and Paykit 0.1.0. A debug fixture provisioned disposable native identities and displayed call/state evidence. The real deletion workflow and SDK ran against a local homeserver fault proxy; no SDK result was replaced.
Test 1: Manual 1 / J1 — private cleanup fails ✅ Failure appears before contact or profile deletion.
A native shared-state read returned 503 during private cleanup. After Cancel, the inspector and call trace confirmed the session, contacts, overrides and sharing settings remained intact, with no contact or profile deletion entered.
endpoint-2x.2x.mp4
Private cleanup failure and retained state, at 2× speed.
Test 2: Manual 1 / J1 — contact write is unconfirmed ✅ Local evidence survives and profile deletion is not entered.
The proxy accepted the native write-ahead record, then rejected shared-state publication with 503. The SDK reported an unconfirmed write. The failure dialog appeared, both local contacts and overrides remained, and the profile-delete counter stayed at zero.
contact-final-2x.2x.mp4
Unconfirmed native contact cleanup, at 2× speed.
Test 3: Manual 1 / J1 — partial native removal ✅ Only the acknowledged contact and override disappear.
The native SDK retained a contact with a published public marker and acknowledged the other removal. The remaining contact and override survived; the app showed failure without entering profile deletion. The marker was provisioned through the native SDK.
partial-2x.2x.mp4
Partial removal and retained evidence, at 2× speed.
Test 4: Manual 2 — profile deletion returns 404 ✅ Failure preserves the session and sharing settings.
The request trace confirmed profile GET 200 followed by DELETE 404. The app kept its failure UI, authenticated session and both sharing settings. Earlier acknowledged contact removals remained applied.
profile404-2x.2x.mp4
Rejected profile deletion and retained session, at 2× speed.
Test 5: J1 — Cancel and cold restart ✅ Normal app startup restores the same identity without replaying profile deletion.
After unconfirmed contact cleanup and Cancel, a normal app launch restored the same identity on Home. A fresh UI snapshot also confirmed the Profile screen. The native trace recorded a successful profile read and no profile DELETE during the 60-second observation. A separate cold profile-manager initialization after Test 4 also restored its identity without replay.
normal-startup-final-2x.2x.mp4
Cold launch through the normal app, at 2× speed.
Check 1: focused native regression checks ✅ The unmodified head passes the affected suites and formatting.
ContactsManagerTests and PubkyProfileManagerTests passed. SwiftFormat 0.64.0 passed the changed Swift files, and git diff --check passed. This does not replace the full hosted suite: its existing SendConfirmation lifecycle failure remains outside these focused checks.
Full review of d1745fb..a062618, all seven changed files. The manager's default cleanup entry point and local missing-file deletion error had source/unit coverage rather than separate device cases. Active-recurring and peer-lease retention variants were not separately driven; the published-marker case exercised the native partial-result branch. Manual Retry/Disconnect ownership and cross-device mutation safety remain deferred by this PR.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup matches the cleanup/error propagation behavior in the reviewed source; Android's local finalization differs.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full review of the complete PR diff against merge base d1745fb, at a062618. No saved assessment baseline was available.
No new actionable code findings.
Incomplete private-endpoint withdrawal, a failed contact read, a partial contact removal, and an unconfirmed profile-deletion error stop Edit Profile deletion before sign-out. Contacts and overrides that were not acknowledged as removed stay in place. Confirmed profile absence still completes: PaykitSdkService.deletePaykitProfile returns when fetchPaykitProfile is nil, and sign-out follows that return.
Parity with bitkit-android#1690 at 2daec90 matches on stopping deletion after incomplete cleanup and on keeping the session after a 404-like profile deletion error. When the remote contact list is empty, Android returns without clearing local-only contacts; iOS still clears that local-only state after the empty read succeeds.
bitkit-ios#1122 at ca10ace was checked only for the no-replay contract this branch is based on, not as a full review. Profile-deletion sources at that head match this PR's base. Profile deletion still runs only from the Edit Profile confirmation and its Retry action. Retry and Sign out are still not bound to the sign-in that started the attempt. That limitation remains bitkit-android#1687.
Validation: inspected PubkyProfileManagerTests and ContactsManagerTests at this head and did not execute them. SwiftFormat passed. Unit tests failed in SendConfirmationLifecycleTests.testPendingWalletSwitchResumesAfterTags (confirm versus utxoSelection). That file is outside this diff and was not re-run here.
Device testing: not performed in this review. An earlier simulator pass at this same head reports the failure cases; those results are that reviewer's.
Suggested additional test cases
- iOS simulator. Disposable signed-in profile with one saved contact and private sharing on, and no homeserver fault. Confirm deletion from Edit Profile. Expected: no failure dialog, the session ends, the app navigates to profile choice or intro, and the contact is gone.
Reviewer model: grok-4.7
Reasoning effort: high
|
Device gate (partial):
Still to check before the final line: after a refused-connection attempt, the next deletion in the same process failed in 1.4 s with no fault active and no new homeserver connection, and background refreshes failed with the same error until a relaunch. It may predate this PR; the base build hasn't been run yet. The faulted attempt logged the same error (below); the unfaulted one followed at 10:27:59 → 10:28:00.732 UTC with no proxy CONNECT in between. |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 7e172c6 after the comparison base changed from d1745fb to 7f54d7b. No inherited coverage. Merge base is 7f54d7b.
No new actionable code findings.
Incomplete private-endpoint withdrawal, a failed contact read, a partial contact removal, and an unconfirmed profile-deletion error stop Edit Profile deletion before sign-out. Contacts and overrides that were not acknowledged as removed stay in place. Confirmed profile absence still completes: PaykitSdkService.deletePaykitProfile returns when fetchPaykitProfile is nil, and sign-out follows that return. Cancel and a later cold start leave profile deletion unstarted. Retry and Sign out still use the current session rather than the attempt that failed. That ownership gap remains bitkit-android#1687.
Parity with bitkit-android#1690 at 2daec90 matches on those stop and preserve outcomes. bitkit-ios#1122 at 7f54d7b was checked only for that no-replay contract, not as a full review.
Validation: inspected PubkyProfileManagerTests.swift and ContactsManagerTests.swift and did not execute them. SwiftFormat passed at this revision. Unit tests were still running. Device testing: not performed in this review. A partial simulator pass at a062618 covers journey actions 1, 2, 3, 7 and 8 only.
Recommended before device testing
Wait for the unit-test run at this revision to finish.
Suggested additional test cases
- iOS simulator. Disposable signed-in profile, one saved contact, private sharing on, and no homeserver fault. Confirm deletion from Edit Profile. Expected: no failure dialog, the session ends, the app opens profile choice or intro, and the contact is gone.
- iOS simulator. Same disposable profile, with a proxy that refuses the private-endpoint connection once and then allows it. Confirm deletion, cancel the failure dialog, clear the fault, and confirm deletion again without relaunching. Expected: the second attempt opens a new homeserver connection, and the first transport error does not block a later successful deletion.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving. The code scan at a062618 had no findings, the only new commit is a merge of the parent branch, and the device gate passed at 7e172c6.
Device gate: 7e172c6, iPhone 17 simulator, a disposable profile with a saved contact on staging, through a local homeserver fault proxy. profile-cleanup-failure.xml actions 1–3, 7 and 8 were driven twice.
- After the private-endpoint withdrawal fails, deletion stops with contacts and profile kept, and "Unable to Delete Profile" offers Retry, Disconnect and Cancel.
- The base
7f54d7b, on the same steps, went pastprivateUnavailableand deleted the contacts and the profile, which is the bug this PR fixes. - The follow-up
acquire Pubky write lockfailure I noted earlier is not from this PR. It reproduces on base too (head 2 of 2, base 1 of 1), clears within about 60 s without a relaunch, and nothing in Bitkit holds the lock. The earlier "until relaunch" reading came from a run that re-applied the fault. Paykit 0.1.1-rc2 now logs two causes:- HTTP 405: pubky-sdk 0.16.2 caches a failed
/infofetch as an empty feature list for 60 s and falls back to the legacy LOCK route, which the homeserver rejects. - 423
shared_state_busy: a lock granted just as the tunnel was cut keeps its 60 s server lease because the unlock never arrived.
- HTTP 405: pubky-sdk 0.16.2 caches a failed
- Perf: not relevant.
Run Tests was still pending at 7e172c6 when I posted; confirm it's green before merge.
There was a problem hiding this comment.
Verdict:
Findings: None in the code inspected.
QA for c18e747: iPhone 17 simulator, iOS 26.5. A debug fixture provisioned disposable identities and contacts and displayed call counters and state; the real deletion workflow and native SDK ran against a controlled homeserver. The fixture did not replace SDK results.
Test 1: Private cleanup failure ✅ A shared-state read returned 503 and deletion stopped before contacts or profile were deleted.
The request trace confirmed the 503; the inspector showed the session, two contacts, two overrides and sharing settings retained.
private-2x.2x.mp4
Private cleanup failure, at 2× speed.
Test 2: Unconfirmed contact removal ✅ An unconfirmed native write preserved both contacts and stopped profile deletion.
The request trace showed the write-ahead record accepted and shared-state publication rejected with 503. The inspector showed both contacts and overrides retained.
unknown-2x.2x.mp4
Unconfirmed contact cleanup, at 2× speed.
Test 3: Partial removal ✅ The retained contact and override survived and profile deletion did not begin.
A native public marker retained one contact; the inspector showed one contact and override remaining after the other was removed.
partial-2x.2x.mp4
Partial removal, at 2× speed.
Test 4: Profile deletion returns 404 ✅ Failure preserved the session and sharing settings.
The request trace showed profile GET 200 followed by DELETE 404. Previously acknowledged contact removals remained applied.
profile404-2x.2x.mp4
Rejected profile deletion, at 2× speed.
Test 5: Cancel and normal restart ✅ The normal app restored the same profile without replaying deletion during the observed startup.
After an unconfirmed contact write and Cancel, normal startup restored the identity; the request trace showed no profile DELETE during the observation.
normal-2x.2x.mp4
Cancel and normal startup, at 2× speed.
Test 6: Recovery after connection failure ✅ A second deletion completed after connectivity returned without an app relaunch.
The fixture showed failure before cleanup, then the success route and cleared session and contacts after the connection was restored.
refusal2-2x.2x.mp4
Connection interruption and recovery, at 2× speed.
Test 7: Successful deletion with a saved contact ⏭️ The normal app completed deletion, but contact presence immediately before the attempt was not established.
The trace showed profile DELETE 204 and the normal app opened the profile introduction. The earlier unconfirmed contact write could have reconciled on restart. A separate fixture run started with two contacts and reached its success route after cleanup, but it does not prove the normal-app precondition.
success-2x.2x.mp4
Normal-app deletion, at 2× speed.
recovery-2x.2x.mp4
Fixture cleanup and success, at 2× speed.
The earlier authenticated review covered these fault paths at a062618; the Paykit dependency changed since then. The later device report covers private cleanup and restart at 7e172c6. Current-head hosted tests and formatting passed.
Reviewed the deletion interactions and scanned the main production changes in a062618..c18e747; this did not exhaust every changed test, journey, localization, or dependency implementation. The device cases did not exercise the unrelated merged USDT, backup, or payment changes.
Twin: bitkit-android#1690 matches the stop-on-incomplete-cleanup behavior; local finalization differs.
Test fixture
Not pushed. Provisions disposable identities and observes native calls without replacing their results.
diff --git a/Bitkit/BitkitApp.swift b/Bitkit/BitkitApp.swift
index 961935aa..12b4fcb1 100644
--- a/Bitkit/BitkitApp.swift
+++ b/Bitkit/BitkitApp.swift
@@ -184,7 +184,9 @@ struct BitkitApp: App {
var body: some Scene {
WindowGroup {
#if DEBUG
- if ProcessInfo.processInfo.arguments.contains("-contact-import-ui-test") {
+ if ProcessInfo.processInfo.arguments.contains("-profile-cleanup-probe") { // test hook
+ ProfileCleanupProbeView() // test hook
+ } else if ProcessInfo.processInfo.arguments.contains("-contact-import-ui-test") {
ContactImportUITestFixture()
} else if ProcessInfo.processInfo.arguments.contains("-profile-recovery-ui-test") {
ProfileRecoveryUITestFixture()
diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift
index 7be9cb33..1bc3ad58 100644
--- a/Bitkit/Services/PubkyService.swift
+++ b/Bitkit/Services/PubkyService.swift
@@ -1847,9 +1847,22 @@ actor PaykitSdkService {
private nonisolated static func config() throws -> PaykitSdkConfig {
var config = try Paykit.defaultConfig(appId: "bitkit")
config.publicContactSharing = .privateOnly
+ #if DEBUG
+ if ProcessInfo.processInfo.arguments.contains("-retain-public-contact") { // test hook
+ config.publicContactSharing = .enabled // test hook: permit native marker provisioning
+ }
+ #endif
return config
}
+ #if DEBUG
+ func reviewPublishContact(_ key: String) async throws { // test hook: stage real native retained contact
+ try await withStateRevisionTracking { sdk in // test hook
+ _ = try await sdk.publishPublicContact(publicKey: key) // test hook
+ }
+ }
+ #endif
+
nonisolated static var clientID: String {
switch Env.network {
case .bitcoin: "bitkit.to"
diff --git a/Bitkit/Utilities/Testing/ProfileCleanupProbe.swift b/Bitkit/Utilities/Testing/ProfileCleanupProbe.swift
new file mode 100644
index 0000000..20f6834
--- /dev/null
+++ b/Bitkit/Utilities/Testing/ProfileCleanupProbe.swift
@@ -0,0 +1,145 @@
+#if DEBUG
+import Paykit
+import SwiftUI
+
+// test hook: provisions disposable native data and observes real SDK operations; no result is replaced.
+@MainActor
+final class ProfileCleanupProbe: ObservableObject {
+ static let shared = ProfileCleanupProbe()
+ @Published var status = "Preparing disposable identity"
+ @Published var reads = 0
+ @Published var removals = 0
+ @Published var deletes = 0
+ @Published var evidence = ""
+ @Published var owner = ""
+ @Published var ready = false
+
+ func note(_ message: String) {
+ Logger.info("CLEANUP_PROBE \(message)", context: "ProfileCleanupProbe")
+ }
+}
+
+// test hook: all deletion decisions remain in EditProfileView and the existing managers.
+struct ProfileCleanupProbeView: View {
+ @StateObject private var probe = ProfileCleanupProbe.shared
+ @StateObject private var profile = PubkyProfileManager(profileDeleter: { key in
+ await MainActor.run {
+ ProfileCleanupProbe.shared.deletes += 1
+ ProfileCleanupProbe.shared.note("profile delete entered")
+ }
+ try await PubkyService.deletePaykitProfile(expectedIdentity: key)
+ })
+ @StateObject private var contacts = ContactsManager(contactRecords: {
+ await MainActor.run {
+ ProfileCleanupProbe.shared.reads += 1
+ ProfileCleanupProbe.shared.note("contact read entered")
+ }
+ return try await PubkyService.contactRecords()
+ }, removeContactRecords: { keys in
+ await MainActor.run {
+ ProfileCleanupProbe.shared.removals += 1
+ ProfileCleanupProbe.shared.note("contact remove entered count=\(keys.count)")
+ }
+ let removed = try await PubkyService.removeContacts(publicKeys: keys)
+ await MainActor.run { ProfileCleanupProbe.shared.note("contact remove returned count=\(removed.count)") }
+ return removed
+ })
+ @StateObject private var app = AppViewModel()
+ @StateObject private var navigation = NavigationViewModel()
+
+ var body: some View {
+ VStack(spacing: 4) {
+ CaptionText(probe.status).accessibilityIdentifier("CleanupStatus")
+ CaptionText("Reads \(probe.reads), removals \(probe.removals), deletes \(probe.deletes)")
+ .accessibilityIdentifier("CleanupCounters")
+ CaptionText(probe.evidence).accessibilityIdentifier("CleanupEvidence")
+ Button("Inspect evidence") { inspect() }.accessibilityIdentifier("CleanupInspect")
+ if probe.ready {
+ NavigationStack(path: $navigation.path) {
+ EditProfileView()
+ .navigationDestination(for: Route.self) { _ in CaptionText("Deletion success route") }
+ }
+ }
+ }
+ .environmentObject(profile)
+ .environmentObject(contacts)
+ .environmentObject(app)
+ .environmentObject(navigation)
+ .environment(KeyboardManager()) // test hook: normal Edit Profile environment dependency
+ .preferredColorScheme(.dark)
+ .task { await provision() }
+ }
+
+ private func inspect() {
+ let overrides = ContactsManager.backupContactProfileOverrides()?.count ?? 0
+ let publicSharing = UserDefaults.standard.bool(forKey: PublicPaykitService.publishingEnabledKey)
+ let privateSharing = UserDefaults.standard.bool(forKey: PrivatePaykitService.publishingEnabledKey)
+ probe.evidence = "Session \(profile.currentSession == nil ? "absent" : "retained"); contacts \(contacts.contacts.count); overrides \(overrides); sharing \(publicSharing)/\(privateSharing); deleting \(profile.isDeletingProfile); route \(navigation.path.count)"
+ probe.note(probe.evidence)
+ }
+
+ private func provision() async {
+ guard !probe.ready else { return }
+ probe.note("setup e2e=\(Env.isE2E) localHost=\(Env.pubkyLocalTestnetHost ?? "nil")") // test hook
+ do {
+ if ProcessInfo.processInfo.arguments.contains("-prepare-wallet"), try Keychain.loadString(key: .bip39Mnemonic(index: 0)) == nil { // test hook
+ _ = try StartupHandler.createNewWallet(bip39Passphrase: nil) // test hook: disposable unfunded wallet for normal AppScene startup
+ }
+ if ProcessInfo.processInfo.arguments.contains("-resume-cleanup") { // test hook: invoke real startup without reseeding evidence
+ await profile.initialize()
+ guard profile.currentSession != nil else { throw PubkyServiceError.sessionNotActive }
+ let records = try await PubkyService.contactRecords()
+ contacts.contacts = records.map { PubkyContact(publicKey: PubkyPublicKeyFormat.normalized($0.publicKey) ?? $0.publicKey, profile: .placeholder(publicKey: $0.publicKey)) }
+ probe.owner = profile.publicKey ?? ""
+ probe.status = "Restored through normal profile initialization"
+ inspect()
+ probe.ready = true
+ probe.note("resumed owner=\(probe.owner)")
+ return
+ }
+ if ProcessInfo.processInfo.arguments.contains("-new-cleanup-identity") { // test hook: discard failed test provisioning only
+ await PaykitSdkService.shared.clearState()
+ try Keychain.delete(key: .pubkySecretKey)
+ try Keychain.delete(key: .paykitSession)
+ }
+ let key: String
+ if let stored = try Keychain.loadString(key: .pubkySecretKey) {
+ key = stored
+ _ = try await PubkyService.signIn(secretKeyHex: key)
+ } else {
+ var bytes = [UInt8](repeating: 0, count: 32)
+ guard SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) == errSecSuccess else { return }
+ key = bytes.map { String(format: "%02x", $0) }.joined()
+ try Keychain.upsert(key: .pubkySecretKey, data: Data(key.utf8))
+ let homeserver = try PubkyService.pubkyPublicKeyFromSecret(secretKeyHex: String(repeating: "00", count: 32))
+ _ = try await PubkyService.signUp(secretKeyHex: key, homeserverZ32: homeserver)
+ let publicKey = try PubkyService.pubkyPublicKeyFromSecret(secretKeyHex: key)
+ try await PubkyService.publishPaykitProfile(Paykit.PaykitProfile(displayName: "Disposable cleanup identity", imageUri: nil, extraJson: nil), expectedIdentity: publicKey)
+ let peerKeys = try ["11", "22"].map { try PubkyService.pubkyPublicKeyFromSecret(secretKeyHex: String(repeating: $0, count: 32)) }
+ _ = try await PubkyService.saveContacts(updates: peerKeys.enumerated().map { ContactUpdate(publicKey: $0.element, label: "Peer \($0.offset + 1)") }, expectedIdentity: publicKey)
+ }
+ let publicKey = try PubkyService.pubkyPublicKeyFromSecret(secretKeyHex: key)
+ profile.publicKey = PubkyPublicKeyFormat.normalized(publicKey) ?? publicKey
+ profile.authState = .authenticated
+ profile.profile = PubkyProfile(publicKey: profile.publicKey!, name: "Disposable cleanup identity", bio: "Local testnet", imageUrl: nil, links: [], status: nil)
+ if ProcessInfo.processInfo.arguments.contains("-retain-public-contact") { // test hook
+ let peer = try PubkyService.pubkyPublicKeyFromSecret(secretKeyHex: String(repeating: "22", count: 32))
+ try await PaykitSdkService.shared.reviewPublishContact(peer) // test hook: real native publication
+ }
+ let records = try await PubkyService.contactRecords()
+ contacts.contacts = records.map { PubkyContact(publicKey: PubkyPublicKeyFormat.normalized($0.publicKey) ?? $0.publicKey, profile: .placeholder(publicKey: $0.publicKey)) }
+ ContactsManager.restoreContactProfileOverrides(Dictionary(uniqueKeysWithValues: contacts.contacts.map { ($0.publicKey, PubkyProfileData.from(profile: $0.profile)) }))
+ UserDefaults.standard.set(true, forKey: PublicPaykitService.publishingEnabledKey)
+ UserDefaults.standard.set(true, forKey: PrivatePaykitService.publishingEnabledKey)
+ probe.owner = profile.publicKey!
+ probe.note("ready owner=\(probe.owner) contacts=\(records.count)")
+ probe.status = "Authenticated local testnet identity"
+ inspect()
+ probe.ready = true
+ } catch {
+ probe.status = "Provisioning failed: \(error)"
+ probe.note(probe.status)
+ }
+ }
+}
+#endif
private: [{'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/profile.json', 'status': 404}, {'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/profile.json', 'status': 404}, {'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143485, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143518, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143518, 'delayed_ms': 1, 'held': False, 'method': 'GET', 'owner': 'gytpba9586axacqcbg9k9dkxsq78t9ju75db9ognzfh5a47smsfo', 'path': '/pub/paykit/v0/shared-state.bin', 'status': 503}]
unknown: [{'at': 1793143564, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143564, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143564, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143585, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143585, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143585, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143585, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143585, 'delayed_ms': 1, 'held': False, 'method': 'PUT', 'owner': '9czuate5go5f58m8twfq475od78t9yuofajbcaa67e33ycy1zzqo', 'path': '/pub/paykit/v0/shared-state.bin', 'status': 503}]
profile404: [{'at': 1793143683, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143683, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143683, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143683, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143684, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143684, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143684, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143684, 'delayed_ms': 1, 'held': False, 'method': 'DELETE', 'owner': 'j44b3gpywszx59pq7pn8m619h1x7suapega41p4fs8sm3ouoexao', 'path': '/pub/paykit/profile.json', 'status': 404}]
normal: [{'at': 1793143980, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143980, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143980, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143980, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143981, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143981, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143981, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793143981, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}]
success: [{'at': 1793144015, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144016, 'delayed_ms': 0, 'held': False, 'method': 'DELETE', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/profile.json', 'status': 204}, {'at': 1793144016, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144016, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144016, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144017, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144017, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}, {'at': 1793144017, 'delayed_ms': 0, 'held': False, 'method': 'GET', 'owner': '7j4ekku4586o98o6pwyxsh4uitb84cwmunazkwa9b7ifj6h3w5yy', 'path': '/pub/paykit/v0/shared-state-writes/', 'status': 404}]
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
Twin: synonymdev/bitkit-android#1690
Dependency: #1122
This PR stops profile deletion after incomplete payment-link or contact cleanup instead of discarding the cleanup error and local contact evidence.
Description
Out of Scope
Design
Existing Profile › Profile Edit in the Bitkit handoff. This changes failure handling; it adds no visual design.
Preview
N/A — existing error UI is reused.
QA Notes
Setup
Use this branch's normal released dependencies: Core 0.8.0-rc11, Paykit 0.1.1-rc2 and VSS 0.5.26-rc1. Component tests use synthetic identities and injected service failures; no real profile, wallet or funds are needed. For J1, provision a disposable authenticated identity and a homeserver fault proxy that can hold/fail private endpoint withdrawal, lose a contact-removal acknowledgement, and return a partial removal with a retained active peer, or fail profile deletion with a 404-like error without confirming the outcome. Those controls are unavailable in the standard journey fixture. The committed
journeys/profile/profile-cleanup-failure.xmldescribes the sequence; do not use a personal profile or reset another worker's wallet.Journeys
profile-cleanup-failure.xml— incomplete cleanup preserves retained evidence and prevents later profile deletion.Manual Tests
1. External reviewer reported native authenticated private-cleanup, unconfirmed contact-write and partial-removal cases at
a0626189; failure appeared before profile deletion and retained evidence stayed intact. See the scoped review evidence below.2. External reviewer reported profile GET 200 followed by DELETE 404 at
a0626189; failure UI, session and sharing settings remained. See the scoped review evidence below.Automated Checks
PubkyProfileManagerTests.swift— the actual view workflow stops after private/contact cleanup failure, retains the session/evidence and releases deletion admission.ContactsManagerTests.swift— unavailable contact reads preserve local evidence; existing partial-removal coverage remains in place.Evidence
Current composition
c18e7475adopts live parent #1122 at162ea52bthrough a signed additive merge; the seven-file cleanup patch is byte-identical. Final combined-tree validation passed 2,988 selected unit tests (zero failures/skips) and 39 UI executions with three disabled Trezor skips. SwiftFormat: 0/852; translations: zero errors and 15 warnings verified against the actual parent. Complete logs and all 3,030 nested test summaries were inspected. Four unit and 11 UI QoS observations and a recovered second tap remain documented without a causal claim. Two independent scoped reviews are clean; all five live-base commits are locally and GitHub verified. New-head hosted checks are pending. This composition does not re-run the authenticated cleanup journey or make the prior failure/reviewer evidence apply to a new runtime.Controlled baseline after behavior-preserving view-helper extraction failed for both cleanup stages: profile deletion was entered and contact evidence erased. A separate actual contact-manager baseline failed when a read error mentioning 404/missing was treated as empty. The repaired focused suites passed 133 profile-manager and 94 contact-manager cases. Final native validation passed 2,908 unit tests with zero failures/skips, and full SwiftFormat 0.64.0 checked 847 files with zero violations. Complete build and nested result diagnostics were inspected: two AppIntents notices and four runtime QoS observations remain; their timing cause is unproven. Two consecutive independent final reviews found no outstanding issues in this scoped change.
The view now uses the throwing contact cleanup; the unused best-effort wrapper was removed. The manager's default private-cleanup route also propagates failure. This change adds no automatic replay of profile deletion; existing endpoint reconciliation remains outside this PR’s scope.
The follow-up at
a0626189removes broad profile-deletion error suppression. Its controlled pre-fix test produced false success, cleared session and sharing state for both a 404-like error and a local missing-file error. The repaired table also checks normal completion. One behavioral test replaces two obsolete classifier tests. Final validation passed 2,907 unit tests with zero failures/skips, and full SwiftFormat checked 847 files with zero violations. Complete logs and nested XCResult issues were inspected: two AppIntents notices and five runtime QoS observations remain, with unproven timing cause. Two consecutive independent final reviews are clean. The SDK already represents confirmed profile absence as normal completion; arbitrary thrown errors no longer authorize sign-out. Earlier acknowledged contact/endpoint removals are not rolled back.Gaps
The complete J1 matrix has not been independently executed by this worker. The external reviewer supplied authenticated coverage for the manual cases and a cold restart, as detailed below. The default manager cleanup entry and local missing-file case have source/unit coverage; active-recurring and peer-lease variants were not separately driven. Neither those results nor component tests establish cross-device CAS, safe manual Retry/Disconnect ownership or historical incident causation. All five commits in the current live-base PR range are verified locally and on GitHub (
verified=true,reason=valid). The earlier second commit only renamed unchanged changelog bytes to this PR number. The earliera0626189hosted failure remains preserved below; the new composition's hosted results are pending. Prior-head formatter40162560144passed 0/847 files; its complete 193-line log was inspected.Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)Latest hosted failure investigation
Prior-head
a0626189run 40165956252 failed onlySendConfirmationLifecycleTests.testPendingWalletSwitchResumesAfterTagsat the three-second destination-appearance assertion. All later wallet/preparation/navigation-path assertions and all 39 UI executions passed; one native-fixture unit and four UI fixtures were skipped. Complete logs and SHA-256/CRC-verified XCResults, including nested issue groups, were inspected. Both this run and the paired profile PR reproduce a keyboard queue timeout after the tag input is removed before keyboard presentation completes. The precise causal ordering and a safe repair remain unproven. A subsequent detached diagnostic checkout atd1745fb9, with trace/failure-capture additions and unchanged product behavior/assertions, passed 2,906 selected tests on a fresh iPhone 16 simulator using CI's exact iOS 26.2 build23C54and Xcode 16.4. The affected case completed in 0.737 seconds. Complete local logs and nested XCResult issues were inspected; four QoS observations remain. Local macOS 27 and hardware/load still differ from the hosted macOS 15 arm64 runner. This comparison does not repair the hosted failure or validate the current PR head. No deadline was weakened and no unchanged CI retry was requested. Existing local full-suite passes remain revision-scoped evidence, not a green hosted result.External authenticated review evidence — October 26
Review 5653598069 reports five controlled cases on the exact
a0626189head, using iPhone 17/SE simulators on iOS 26.5 with Core 0.8.0-rc11 and Paykit 0.1.0. A debug fixture provisioned disposable native identities and displayed call/state evidence; the real deletion workflow and SDK used a local homeserver fault proxy without substituted SDK results. Private-read 503, write-ahead acceptance followed by rejected shared-state publication, partial removal with a published marker, and GET 200/DELETE 404 preserved the appropriate session/contact/sharing evidence and stopped later deletion. Cancel plus a normal cold launch restored the same identity, with no profile DELETE during the reported 60-second observation. Videos and trace descriptions are linked in that review.These are attributed reviewer results, not a new run by this worker or proof of every J1 variant. The reviewer explicitly retains the current hosted lifecycle failure and deferred ownership/CAS limitations. Ovi subsequently approved the scoped change; that approval does not resolve the unrelated CI failure.
Current parent composition — October 27
Signed additive merge
7e172c61240b368cb12a4a63a2b1f6e9ded753b9incorporates live #11227f54d7b9f7a224a6da3c726a6f0a6fa04bb3083ewithout rewriting history. The cleanup patch remains intact; the overlapping contact files also retain the parent's temporary refresh-error handling. Final combined-tree validation passed 2,966 selected units with zero failures/skips, 39 UI executions with three explicit Trezor skips, and SwiftFormat 0.64 with 0/849 findings. The expired pinned-fork UI case and documented integration fixtures were excluded, not counted as passes. All 1,680 frozen inputs matched and two consecutive independent final reviews were clean.Complete 30,248-line unit and 4,764-line UI logs, modern/legacy summaries and every nested test/activity record were inspected. Four unit QoS stacks match saved parent observations; the UI result retains 16 unsymbolized/zero-address warnings across 14 executions. Their timing cause is unproven and the counts do not establish a regression or repair. AppIntents and device-enumeration notices are retained. The earlier hosted route failure remains unwaived; this local pass is not a causal diagnosis. Full four-commit live-base signatures are valid locally and on GitHub. New-head hosted CI and complete authenticated J1 acceptance remain to be inspected/completed; no automatic destructive replay or contact CAS claim is added.