Repository navigation
fix: stop deletion after incomplete cleanup - #1124
ben-kaufman wants to merge 17 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
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 c18e747 after the merge base changed since the saved assessment. No inherited coverage. Merge base is 162ea52.
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 use the session that is current when they are tapped. That ownership gap remains bitkit-android#1687.
Parity with bitkit-android#1690 at 2daec90 matches on those stop and preserve outcomes. After the remote contact list is fully removed, or the read succeeds and is empty, iOS clears remaining local-only contact state before profile deletion; Android removes only the acknowledged remote contacts. bitkit-ios#1122 at 162ea52 was checked only for the no-replay contract: deleteProfileWithSessionRetry is absent, and profile deletion still runs from the Edit Profile confirmation and its Retry action.
Validation: inspected PubkyProfileManagerTests.swift and ContactsManagerTests.swift and did not execute them. SwiftFormat and unit tests passed at this revision.
Device testing: not performed in this review. A simulator pass at this revision reports the failure and recovery cases; contact presence immediately before the successful deletion was not established. An earlier partial journey pass at 7e172c6 covers actions 1–3, 7 and 8 only.
Ready for device testing
Suggested additional test cases
- iOS simulator. Disposable signed-in profile, one saved contact still present, 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 that contact is gone.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of c26d7b13 against merge base f3df6cda after the comparison base changed. The complete PR diff and affected callers/tests were inspected; no coverage was inherited.
No new actionable code findings.
Cleanup failures stop subsequent deletion, and unacknowledged contacts/overrides remain available. Confirmed profile absence still completes through the service's optional profile result. Targeted comparisons with Android #1690 at 2daec90 and iOS #1122 at f3df6cda checked cleanup/error propagation and the absence of automatic deletion replay; neither comparison was a separate full review. Paykit contracts were checked at 01de5dd.
Existing concern: Retry and Disconnect still use the sign-in current when tapped; the ownership gap tracked in #1687 remains outside this repair.
Validation: hosted tests and SwiftFormat passed on merge 3d572813, containing the exact reviewed head/base. Logs confirm the new cleanup-retention and deletion-acknowledgement cases passed. Locally, diff whitespace checks and journey XML validation passed; native tests were inspected but not executed because Xcode/Swift are unavailable.
Device testing: not performed in this review. Earlier fault-injection results at c18e747 remain revision-bound; J1 needs its controlled proxy on this composition.
Suggested additional test cases
- iOS: use a disposable authenticated profile with private sharing enabled and one saved contact confirmed present immediately before deletion, with no injected fault. Delete from Edit Profile; expect the session and contact to clear, profile choice/intro navigation, and no failure dialog.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
|
@ben-kaufman Moving this PR to 3.2.0 per the 3.1.0 cut line (https://synonymworkspace.slack.com/archives/C07BJ7DNPCG/p1793307568629859): the restore and profile-deletion stacks harden behaviour that already shipped in 3.0, so they keep landing on master and ship in 3.2. |
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for b14d5a1: iPhone 17 simulator, iOS 26.5; Paykit 0.1.1-rc2 and bitkit-core 0.8.0-rc11. A debug fixture provisioned disposable authenticated identities, encrypted links and accepted native subscriptions. The actual EditProfileView and native SDK handled removal; no deletion decision or SDK result was substituted.
Fixed-term subscriptions show their latest expiry ✅ Deletion is refused with the later expiry date, before any cleanup.
Native setup logs confirmed a payee subscription ending 7 November and a payer subscription ending 14 November. The warning showed “Expires 14 November 2026”. The recorded inspector confirmed zero private-withdrawal, contact-read, contact-removal and profile-delete entries, with the session, three local contacts and overrides, and both sharing settings retained.
finite2x.2x.mp4
Latest finite expiry and retained state, at 2× speed.
Open-ended subscriptions suppress expiry guidance ✅ Adding an open-ended subscription removes the date while continuing to refuse cleanup.
Native setup logs confirmed an accepted open-ended payer subscription alongside both fixed-term subscriptions. The warning contained no expiry date or generic Retry dialog. The recorded inspector showed all cleanup counters still at zero, with the session, existing local contacts and overrides, and sharing settings retained. Native contact storage also retained the newly provisioned peer.
mixed2x.2x.mp4
Mixed subscription terms and refusal without a date, at 2× speed.
Check: Native regressions and translations ✅ The unmodified head builds and the affected suites pass.
xcodebuildmcp simulator build built the pinned head. xcodebuild test passed PaykitSdkIdentityCheckTests, PubkyProfileManagerTests and ContactsManagerTests, including finite/unknown expiry and failed/cancelled subscription reads. SwiftFormat passed the seven PR Swift files; translation validation and diff whitespace checks passed. The test log retains a QoS warning and AppIntents metadata notices. The full suite was not rerun locally.
Other J1 cleanup, Cancel/restart and healthy fresh-confirmation cases were not repeated: I relied on the author’s current-head authenticated acceptance report. The previous authenticated review at a2b2186 supplies the unavailable-subscription-state and transport-recovery results; their failed-read propagation and explicit retry behavior are unchanged. These are attributed results, separate from the two device cases above. Complete app runtime logs from this run were not retained; the recordings, snapshots and captured native setup observations support these two results.
Reviewed the 108-file contiguous a2b2186..b14d5a1 delta, focusing on expiry guidance, reached deletion callers and interactions with merged parent changes. The device cases did not exercise unrelated inherited payment, backup or CI behavior. Malformed expiry and alternate manager entry points have source/unit coverage; Ring unlink was not driven separately. Original-attempt Retry/Disconnect ownership and cross-device conditional cleanup remain explicitly deferred by this PR.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup at 815e85c matches subscription refusal and latest-expiry selection; iOS also checks empty strict contact batches, and local finalization differs.
Reviewed by gpt-6.1-sol-high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the complete PR diff at b14d5a1 against merge base bee8975a, after the comparison base changed from 940d7f3 to bee8975 and no coverage baseline was supplied. No coverage was inherited.
No new actionable code findings.
Edit Profile deletion and unlink refuse an active payer, payee, or contactless subscription before private-endpoint withdrawal. The same active-recurring check runs again at strict contact removal and profile deletion. The expiry line is the latest finite end only when every blocking record has a parsed end; an open-ended or unparseable end stays a neutral refusal. A failed withdrawal, contact read, or partial acknowledgement stops sign-out and leaves unacknowledged contacts and overrides in place. A thrown profile-deletion error, including 404 text, keeps the session and sharing flags. Confirmed absence still completes when profile fetch returns nil.
The shared cleanup and subscription contract matches bitkit-android#1690 at 815e85c0, including journeys/profile/profile-cleanup-failure.xml. iOS also checks an empty contact batch and repeats the check inside profile deletion. bitkit-ios#1122 at bee8975 was inspected for deletion admission and that confirmed-absence path.
Non-blocking consistency suggestions. iOS formats the refusal expiry as a long local date with the time omitted, matching the existing iOS contact-deletion toasts. Android’s profile toast uses toLocalizedTimestamp, which includes the time. Both platforms still block until the exact end instant.
Validation was source inspection of the pinned diff, callers, and unit tests. Those tests were not executed. Device testing: not performed in this review. Safe manual Retry/Disconnect after uncertain cleanup remains bitkit-android#1687.
Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to b14d5a1. No new findings.
a2b218613 and 6665a4431 refuse profile removal while a recurring subscription is active (endsAt null, unparseable or in the future). This applies as payer and payee, with or without a contact. The toast shows the latest end date only when every blocker has a parsable end. Open-ended subscriptions can be ended in-app by either role, and wallet wipe doesn't go through this check. The rule matches synonymdev/bitkit-android#1690. The only merge with a conflict, 2df5bad1, resolved a test-file conflict by keeping both tests.
My approval is pending a device run of the new behaviour.
Device gate (partial): b14d5a1, not run yet. Pending: profile-cleanup-failure.xml with two linked wallets (actions 1–3), and actions 4–11 with the homeserver fault proxy. l10n: profile__delete_active_subscription changed in every locale.
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for de683f3:
Check: Cleanup and queued publication regressions ✅ The unmodified head builds and the focused native suites pass.
Built with xcodebuildmcp simulator build and ran the selected xcodebuildmcp simulator test suites on an iPhone 17 simulator, iOS 26.5. Cleanup failures, subscription refusal, SDK queue admission, session replacement, public/private reconciliation and payment-request regressions passed without failures or skips. The native result and raw log confirm the pass, including one method omitted from the wrapper’s case list. SwiftFormat 0.64.0, translation validation and diff whitespace checks passed. QoS runtime warnings and an AppIntents metadata notice remain in the evidence; no warning-free claim. The full hosted suite was still running when this review ended.
Check: Earlier profile-cleanup device results ✅ The delta preserves the paths covered by the passing J1 reports.
No device journey was repeated. EditProfileView, both cleanup managers, the subscription/contact-removal checks, the journey and the native dependency pins are unchanged from b14d5a1. The private-cleanup change forwards a session check to app registration, which profile deletion skips through deferAppSync: true; the SDK revision-tracking extraction preserves admission and error propagation.
The b14d5a1 device report proves latest finite expiry and open-ended refusal. The authenticated a2b2186 report covers payer/payee refusal, unavailable subscription state, private withdrawal failure, unknown/partial contact removal, rejected profile deletion, Cancel/restart and fresh-confirmation recovery. The saved-contact success report and the author’s authenticated acceptance report supply the normal-app success and later J1 matrix evidence. These remain attributed results at their stated revisions, not new device passes on de683f3.
Reviewed the contiguous b14d5a1..de683f3 delta, concentrating on strict deletion’s interaction with the merged parent’s SDK admission and endpoint reconciliation. Unrelated inherited payment, backup and CI changes were not driven on devices. Alternate manager entry points and malformed expiry retain source/unit coverage; Ring unlink was not separately driven. Original-attempt Retry/Disconnect ownership and cross-device conditional cleanup remain explicitly deferred.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup at 7ef3432 matches subscription refusal, latest-expiry selection and stopping on incomplete cleanup. iOS additionally checks empty strict contact batches; local finalization differs.
Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for dd180c6:
Check: Cleanup, recovery and SDK admission regressions ✅ The unmodified head builds and the selected native suites pass.
Built with xcodebuildmcp simulator build and ran xcodebuildmcp simulator test on iPhone 17 / iOS 26.5 with Xcode 27.0. Cleanup, subscription refusal, profile recovery, SDK queue/session admission, private/public reconciliation, allowance admission and send lifecycle checks passed without failures or skips. The native result bundle and raw execution log agree. The new pending-profile recovery and queued-acceptance controls passed. SwiftFormat 0.64.0, translation validation, journey XML parsing and diff whitespace checks passed. QoS runtime warnings and AppIntents metadata notices remain in the evidence. The full local unit/UI and integration suites were not rerun.
Check: Earlier profile-cleanup device results ✅ The delta preserves the paths covered by the passing J1 reports.
No device journey was repeated. The deletion workflow, strict contact/subscription checks, private cleanup, journey and native pins are unchanged. The profile recovery change suppresses remote refetch only while profile setup is pending; it leaves restoration after cleanup failure intact.
The finite/open-ended subscription report, authenticated cleanup and recovery report and normal-app saved-contact deletion report cover J1 and manual cases 1–2. These remain attributed device results at their stated revisions, rather than fresh device passes on dd180c6.
Reviewed the contiguous de683f3..dd180c6 delta and reached cleanup/session callers. The child patch is unchanged against the updated parent. Inherited allowance-toggle, Request Payment and USDT presentation changes received source inspection; no device case drove them. Ring-only unlink and alternate manager entry points retain source/unit coverage. Original-attempt Retry/Disconnect ownership and cross-device conditional cleanup remain explicitly deferred.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup at 7ef3432 matches subscription refusal, expiry selection and stopping after incomplete cleanup. iOS also checks empty strict batches; local finalization differs.
Reviewed by gpt-6.1-sol-high
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Device gate result. Code findings from my review at b14d5a1 still apply; the PR-owned diff is unchanged at dd180c6 (the delta is parent and master merges only).
Device gate: dd180c6 — profile/profile-cleanup-failure.xml on iPhone 17 and iPhone 17e simulators, two linked wallets (A payee, B payer) with an active open-ended monthly subscription paid for one period, homeserver faults through a local TLS-passthrough proxy. perf: n/a. l10n: profile__delete_active_subscription checked in en, de, el, fr, es, ru and pt-BR on iPhone 17; every string wraps to two lines with no truncation.
- Action 1, refusal with an active subscription: pass for the open-ended case, as payee and as payer. The toast reads "Active subscriptions prevent removing this profile." and nothing after
requireNoActiveSubscriptionsruns. - Action 2, no Retry dialog and no cleanup: pass. No private list clearing, contact removal or profile deletion; both wallets keep their contacts.
- Action 3, deletion after ending the subscription: pass. After B cancelled (detail still shows "Active, EXPIRES 1 December"), deletion ran the full cleanup through sign-out.
- Action 4, fault on before deletion: pass. It fails after the subscription check with
acquire Pubky write lock (connection failure). - Action 5, failure dialog: pass. Retry, Disconnect and Cancel; no success toast and no navigation.
- Action 6, private withdrawal failure: pass. Deletion stops at
list pending Pubky shared-state writes; no contact or profile deletion follows. - Action 9, uncertain profile deletion: pass for the transport-loss variant. The DELETE reached the server and its response was dropped. The app showed the failure dialog, kept the session and the sharing toggles, and replayed nothing after relaunch.
- Action 10, Cancel: pass. No second deletion in the following 40 s.
- Action 11, restart: pass. Two relaunches restored the session with no deletion lines in 70 s.
Skipped:
⚠️ Action 7 (unknown contact removal): not run. The re-created profile only had one-sided contacts, so the private stage stopped first withprivateUnavailable. It needs a linked counterparty online, and no third device was free.⚠️ Action 8 (partial contact removal): not run.removeContactsAndBlockPeersis one SDK write, so a partial acknowledgement cannot be produced from the network.⚠️ The finite-end, mixed and unparseable subscription cases, and the date linesubscriptions__expires_date: not run. Bitkit's create form always sendsendsAt: niland cancelling sets.canceled, so the app cannot reach them. They need a third-party issuer.⚠️ A literal "404 Not Found" response on profile deletion was not forged (TLS); only the dropped-response variant ran.⚠️ The subscription "without a contact" case: not run. Contact deletion is blocked while a subscription with that contact is active, so I found no UI path to that state.
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 dd180c6 against merge base a6647db2 because the comparison base changed from bee8975 to a6647db. No earlier coverage was inherited.
No new actionable code findings.
Edit Profile deletion now checks for active subscriptions before it withdraws private endpoints. The check covers both payer and payee roles and subscriptions with no saved contact. It runs again during strict contact removal and inside deletePaykitProfile, so a known subscription can no longer cause its contact to be silently filtered out while other contacts are deleted. Failures in private-endpoint withdrawal, the contact read, or partial contact removal now stop the flow before profile deletion. In each case the session stays signed in, and contacts and overrides that were not confirmed as removed stay in place. A profile-deletion error, including one with 404 text, keeps the session and sharing settings. Confirmed absence still signs out: at the pinned paykit-rs 01de5dd2, fetch_paykit_profile returns nil for a missing profile, and contact_records only reads local storage, so it has no 404 path. That makes the removed missing-data fallbacks unnecessary in supported flows. The refusal toast is localized in all 15 locales.
The cleanup and subscription contract matches the twin bitkit-android#1690 at 9bf19f19, including journeys/profile/profile-cleanup-failure.xml. The dependency bitkit-ios#1122 has moved to 2f80b445. Its change since this PR's base only adds an admission hook to allowance acceptance and does not overlap this PR's hunks or deletion contracts. Only that compatibility was checked; #1122 was not fully reviewed.
Validation: I inspected the pinned diff, its callers, the SDK source and the updated unit tests. I did not run the tests myself. The current-head Run Tests and SwiftFormat checks passed. Device testing was not performed in this review. A reviewer reports a device gate at dd180c6 that covered most journey actions. It skipped unknown or partial contact removal, finite or unparseable subscription ends, and a literal 404 response. Making a manual Retry/Disconnect safe after an uncertain cleanup is tracked separately in bitkit-android#1687.
Reviewer model: claude-opus-5-5
Reasoning effort: high
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for 3991f3a:
Check: Cleanup, queued allowance and channel-closure regressions ✅ The unmodified head builds and the focused native suites pass.
Built with xcodebuildmcp simulator build and ran selected xcodebuildmcp simulator test suites on iPhone 17 / iOS 26.5 with Xcode 27.0. Cleanup, subscription refusal, SDK queue/session admission, allowance acceptance, receive-invoice publication and payment-proof checks passed without failures, skips or test retries. The native result bundle and raw execution log agree. The queued-offer pause/reactivation regression and last-channel public-invoice withdrawal regression passed, as did the three updated billing-period cases.
SwiftFormat 0.64.4, diff whitespace and journey XML checks passed. QoS runtime warnings and AppIntents metadata notices remain in the evidence. The full local unit/UI and integration suites were not rerun. These are component checks, not new authenticated device journeys.
Check: Earlier profile-cleanup device results ✅ Previously tested cleanup paths are unchanged.
No cleanup journey was repeated. The complete cleanup patch is unchanged against the updated parent, as are the deletion workflow, strict contact/subscription checks, private cleanup, journey and native dependency pins. I retained these attributed results at their recorded revisions:
- Finite and open-ended subscription refusal at b14d5a1.
- Authenticated payer/payee/orphan refusal, unavailable subscription state, private withdrawal, unknown/partial contact removal, DELETE 404, Cancel/restart and fresh-confirmation recovery at a2b2186.
- Normal-app deletion with saved contacts and private sharing enabled at c26d7b1.
- Two-wallet refusal, ended-subscription deletion, private cleanup failure, lost DELETE response and Cancel/restart at dd180c6.
Together these cover J1 and manual cases 1–2. Unknown/partial contact removal, finite/mixed terms, literal DELETE 404 and contactless subscriptions skipped in the latest report have earlier passing reports above; the skips themselves are not passes. Ring-only unlink, malformed expiry and alternate manager entry points retain source/unit coverage rather than separate device claims.
Reviewed the complete eight-file contiguous dd180c6..3991f3a delta and reached cleanup, SDK admission and publication callers. The inherited allowance and channel-closure changes received source/component coverage; no new device journey drove them. Original-attempt Retry/Disconnect ownership and cross-device conditional cleanup remain explicitly deferred.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup at 7040c89 matches subscription refusal, latest-expiry selection and stopping after incomplete cleanup. iOS also checks empty strict batches; local finalization differs. This was a contract comparison, not a full twin review.
Reviewed by gpt-6.1-sol-high
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for ce0784b:
Check: Profile cleanup and backup completion unit suites ✅ The head builds and 279 of 279 selected tests pass, with none skipped.
Ran xcodebuild build-for-testing and test-without-building at the unmodified head on iPhone 17 / iOS 26.5. The PR's suites (PubkyProfileManagerTests, ContactsManagerTests, PaykitSdkIdentityCheckTests) and the two suites the delta changed (BackupForegroundRecoveryTests, BlocktankBackupTests) ran 279 tests: 279 passed, 0 failed, 0 skipped. Counts are from the result bundle, not the wrapper. One QoS priority-inversion runtime warning came from ContactsManagerTests. The full unit and UI suites were not rerun.
Check: Earlier profile-cleanup device results ✅ This push leaves the cleanup code those reports drove unchanged.
I didn't repeat a device journey. After dropping blob ids and hunk offsets, the PR's own 24-file patch is byte-identical against 5a5044a at 3991f3a and against 595898e at ce0784b. The delta changes only BackupService (scheduled-job removal now runs in the same backup-queue step that publishes idle; Blocktank capture reads cached data). ContactsManager, PubkyProfileManager, PubkyService and EditProfileView never reference BackupService, and no code outside it reads backup job state (checked with grep). The same normalized patch is also identical at b14d5a1. The only PR-owned commit between a2b2186 and b14d5a1, 6665a44, changes only the refusal's expiry date and its strings, which the b14d5a1 report drove; the contact-removal and profile-DELETE steps are unchanged since a2b2186. These reports cover every profile-cleanup-failure.xml action and manual cases 1–2:
- Fixed-term latest expiry and open-ended refusal at b14d5a1.
- Payer, payee and orphan refusal, unavailable subscription state, private withdrawal failure, unconfirmed and partial contact removal, DELETE 404, Cancel/restart and fresh-confirmation recovery at a2b2186.
- Normal-app deletion with saved contacts and private sharing at c26d7b1.
- Two-wallet refusal, deletion after the subscription ended, private cleanup failure, lost DELETE response and Cancel/restart at dd180c6.
The author's J1 matrix at b14d5a1, in the PR description, also reports partial and unknown contact removal and DELETE 404 at that patch.
Ring-only unlink, malformed expiry and alternate manager entry points still have source and unit coverage only.
Reviewed the 3991f3a..ce0784b delta: two merges of #1122 (which carries #1123's scheduling fix), with five files changed, all in the parent. No device case drove the parent's backup scheduling or Blocktank capture change; its unit suites passed above. Run Tests CI at ce0784b was still pending when this review was written; SwiftFormat passed. Original-attempt Retry/Disconnect ownership and cross-device conditional cleanup remain deferred, as before.
Twin: bitkit-android#1690 fix: stop deletion after incomplete cleanup moved from 7040c89 to 2e79e10 through master and parent merges only, with no cleanup files changed. It still matches subscription refusal, latest-expiry selection and stopping after incomplete cleanup.
Reviewed by claude-opus-5-5-medium
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for 21d6a22:
Profile copy and QR copy through the system Paste button fixture ✅ The ProfileRecovery UI test passes with the new system Paste button.
Ran ProfileRecoveryUITests on iPhone 17 / iOS 26.5 at the head: 1 of 1 passed. The video shows the fixture's system Paste button reading not-copied after Reset copy, then the copy toast and the pasted profile key, then the failed-load and Try Again states. This answers whether the PasteButton is reachable by its accessibility identifier: it is.
profile-recovery-paste-ui.2x.mp4
Check: Profile cleanup and backup retry unit suites ✅ The head builds and 318 of 318 selected tests pass, with none skipped.
Ran xcodebuild build-for-testing and test-without-building at the head on iPhone 17 / iOS 26.5. This covers the PR's suites (PubkyProfileManagerTests, ContactsManagerTests, PaykitSdkIdentityCheckTests) and the backup suites, including the new BackupServiceTests cases for auth send-failure retry and permission-denied no-retry. Result: 318 passed, 0 failed, 0 skipped. The counts come from the result bundle. The full unit and UI suites were not rerun.
Check: Earlier profile-cleanup device results ✅ This push leaves the cleanup code those reports drove unchanged.
I didn't repeat a device journey. The PR's own patch against b44f025 is identical, line for line, to its patch against 595898e at ce0784b. The delta is exactly #1122's change to four files: BackupService (classifying auth send failures), the DEBUG-only profile recovery fixture and their tests. The attributed device reports listed in the ce0784b review still apply at their recorded revisions.
Not driven: the cleanup journeys (profile-cleanup-failure.xml). This push leaves their code unchanged, and earlier reports cover every action in them. Ring-only unlink, malformed expiry and alternate manager entry points still have source and unit coverage only. Retry/Disconnect ownership of the original attempt and cross-device conditional cleanup are still deferred.
Reviewed the ce0784b..21d6a22 delta. It is #1122's auth transport retry, and those lines belong to #1122's diff, not this PR's. One note for #1122: any AuthError that matches error sending request for url (…) is retried at a capped backoff, and that includes TLS errors. This matches how ConnectionError is handled. The backup retry classification ran only in unit tests, not on a device.
Twin: bitkit-android#1690 at 2e79e10: its own patch is identical, line for line, to the one at 7ef3432 that earlier reviews matched, so it still matches.
Reviewed by claude-opus-5-5-medium
Validated auth transport parent composition — November 3
Signed additive merge
21d6a22036cb0f5d9068b1eb25d346eafd1ea70aincorporates live #1122 parentb44f0253c4b8ecc39e4e38fee2716844e26092da. The four incoming files exactly match that validated parent: typed auth request-send classification, its tests, and the system-Paste DEBUG fixture with unchanged copy assertions. Independent review verified that this PR's own 24-file patch remains byte-identical, including strict cleanup failures, original-session revalidation and contact/endpoint handling.The composed tree passed 2,976 unit tests and five affected UI cases (Profile Recovery plus all four Contact Import cases), with one explicitly unprovisioned USDT fixture skip and zero failures/retries. All three auth controls passed. All 1,693 frozen inputs match; full SwiftFormat reports 0/829, with 108 configured exclusions. Complete raw and nested diagnostics were inspected, and the owner verified 9,091 evidence members. Six QoS observations in unchanged test bodies and two AppIntents notices remain documented; no timing-cause or blanket warning-free claim is made.
All 17 live-base commits verify locally and on GitHub, and
draft=falseis verified. Current-head native CI 41738718686 and SwiftFormat 41738718729 passed on attempt 1. Both jobs used synthetic checkoutbe69f4a4, with the exact published tree and live parent. Canonical results record 2,976 unit passes and 39 UI passes, five explicit fixture skips and zero failures or method retries. Thirty unique locale/appearance launch configurations account for the physical count. The camera, clipboard, three auth controls and both scheduler regressions all passed. Hosted formatting is 0/829, with 108 configured exclusions; nine migration jobs were skipped.Complete logs, nested diagnostics and every result reference were inspected. The artifact digest, all 6,189 archive entries and 9,342 sealed evidence members verify. The 572 canonical linker warnings (286 distinct messages) match retained provenance with identical actual-parent project/lock inputs; simulator/CoreGraphics, keyboard and other runtime notices remain disclosed. Runtime detectors, leak checking and per-test timeouts are disabled in the existing workflow, so this is not a detector-clean claim. The original camera failure and the different scheduling failure in its single authorized retry remain separate historical results. This is validated new-source CI, not another retry. Hosted report:
work/validation/hosted-ios1124-21d6-20261103/report.json, SHA2567f8078b3ff2dbf7aca0494b4dd0e9136224cbdd46810a3b3771addf26564e3e5. Evidence:work/validation/ios1124-auth-parent-final-audit-20261103/report.json(SHA256fc38ee9a7e8fae1f108569d475adcd44c3c474b971c8daa10e5e13f3de287221) andios1124-auth-parent-composition-20261103. This validation adds no new authenticated cleanup, cross-device contact CAS, or terminal complete-WALLET result. Earlier failures, case-specific acceptance and limitations below retain their original revisions.Scheduled-backup completion repair composed — November 3
Signed additive merge
ce0784bdb416ad40cf1c601dd0394ecaaa7f3354incorporates #1122595898e1and #1123’s independently reproduced scheduling fix. Scheduled-job admission is removed before publishing idle in the same serial queue operation. Profile cleanup, required restore gates and cancellation drains retain their existing behavior; the affected composition review found no new interaction.The actual combined tree passed 2,973 unit tests and one Profile Recovery UI test, zero failures/retries, with one opt-in native-fixture skip. The qualified scheduled-idle regression passed. All 1,693 frozen inputs match. SwiftFormat reports 0/829, with 108 exclusions. Complete raw and nested results reconcile the passing case omitted by MCP. Six QoS observations in parent-identical method bodies and two AppIntents notices remain documented, without a timing-cause claim. The owner verified 5,989 diagnostic members. All 16 live-base commits verify locally and on GitHub.
The earlier
e8aaa4b0hosted camera failure and its single authorized retry remain below. That retry passed the camera case but failed a late-retry scheduling wait; its private job-map state was not captured, so the independently reproduced admission gap is not claimed as a proven historical cause. This is a new validated source head, not another unchanged CI retry. New-head hosted native CI 41708815108 passed on attempt 1: 2,973 units and 39 UI executions, zero failures or test retries, with one unit and four UI fixture skips. The camera and late-retry scheduling cases both passed. The synthetic checkout74112f83exactly matches the published tree and live parent595898e1. Hosted SwiftFormat reports 0/829; nine branch-gated migration jobs were skipped, not passed. Complete logs, nested XCResult diagnostics and checksum/CRC-verified artifacts are audited; all 9,243 sealed members verified. The 286 distinct linker warnings match the actual-parent set. These passes do not establish the historical failures' cause. Evidence:work/validation/hosted-ios1124-ce078-20261103/report.json. Existing authenticated/manual and terminal complete-WALLET gaps are unchanged. Evidence:work/validation/ios1124-idle-final-diagnostics-20261103/report.jsonandwork/validation/ios1124-idle-composition-20261103.Historical hosted native failure at
e8aaa4b0— November 3Native run 41675649974, attempt 1 at
e8aaa4b0, failed one UI execution. 2,972 units + 38 UI executions passed, five prerequisite cases skipped, andUsdtWalletUITests.testReceiveAndInvalidPaymentStaySeparateFromBitcoinfailed at line 288: the original camera prompt remained after Allow was tapped and the unchanged five-second dismissal wait. The later manual-entry/invalid-recipient assertions were not reached in that execution. The exact UI file matches actual base #1122/2d604929; this source equality does not establish failure cause. Synthetic checkout0457d6c4exactly matches published treea4ea694c.Complete logs, every nested result, failure hierarchy and captured system diagnostics were inspected; 10,788 evidence members and all archive digests/CRCs were verified. The event targeted the center of the captured Allow button. HID/event-delivery, accessibility automation, simulator diagnostics and a preceding system searchd crash remain recorded; none proves why the prompt persisted. The failure mechanism remains unclassified, and no hosted retry has been made. Formatting passed 0/829, with 108 configured skips; nine migration jobs skipped. The 286 unique linker messages retain their actual-base comparison. Passing local/component results do not waive this hosted failure or supply missing authenticated acceptance. Existing João/Ovi requests remain pending. Evidence:
work/validation/hosted-ios1124-e8aa-20261103/report.json.Cached backup protection propagated — November 3
Signed additive merge
e8aaa4b03c4998a339b119940a67ea63253338bbincorporates live #1122/2d6049298ac2b9081b6f8446b62bdcd0908b93df. Only the three reviewed parent inputs change: cached Blocktank capture, its native SQLite regression tests and the parent changelog. Unreadable cached info now fails capture and retains the acknowledged backup/pending revision; absent cached info remains valid. Released Core nil-ID list reads were already local, so this is not evidence of fixing a network-refresh cause. Cleanup outcomes and session-restoration gates are unchanged.The actual combined tree passed 2,972 unit tests and one ProfileRecovery UI case, with one explicit opt-in native fixture skip, zero failures/retries and formatting 0/829. All 1,693 frozen inputs match. Complete raw logs and all 2,974 nested test summaries were inspected; four QoS observations and two metadata notices are retained, with no introduced source/format finding. Two sealed parent persistence reviews and an independent child interaction review cover this composition. All 15 commits against the live parent verify locally and on GitHub. New-head hosted checks are pending; the prior hosted results below remain historical. This adds no authenticated cleanup J1, remote backup or terminal complete-WALLET acceptance.
Evidence:
work/validation/ios1124-cached-blocktank-composition-20261103,work/validation/ios1124-cached-blocktank-diagnostics-20261103/report.json.Current-head hosted validation — November 2
Hosted native CI at
3991f3a2f8d70cf24412e4ae3251cf956e7965cbpassed 2,970 unit tests and 39 UI executions on iOS Simulator 26.2, with five explicit skips and no failures or test retries. The verified synthetic checkout tree equals the published head. Complete logs and all 3,074 XCResult references were inspected, including 67 attachment occurrences; archive digests/CRCs and every sealed evidence payload were verified. Formatting is 0/828.The 286 distinct linker warnings retain their recorded reference/input comparison. There were 72 invalid-number runtime observations and one keyboard-queue timeout in a passing navigation test; passing tests do not establish their causes. Fixture/hardware skips, migration jobs and excluded integration selectors are not passes. This adds no authenticated profile-removal, wallet-reset or complete-WALLET acceptance. Existing QA cases and limits remain. Evidence:
work/validation/hosted-ios1124-3991-20261101-current-a/NATIVE-REPORT.mdandwork/validation/hosted-four-tail-root-integrity-20261102.json.Validated parent composition — November 1
Signed additive merge
3991f3a2f8d70cf24412e4ae3251cf956e7965cbincorporates live #1122/5a5044a7e8bf711ff63c5ff6e96edafe822eea74. The complete 24-file cleanup patch is unchanged. The final tree6d35816e0f7cc23f49c730ff94a153ac837ec1a9passed 2,970 unit and 39 UI executions, with five explicit skips, no failures and no failure retries. The 30 launch executions cover 15 locales in two appearances; the canonical logical count is 2,980 passes. All 1,692 frozen inputs match. SwiftFormat is 0/828, with 108 configured exclusions.The sole audit inspected complete logs and nested XCResult diagnostics, resolving all 3,075 references and 67 attachments. One passing backoff/jitter test omitted by the wrapper was reconciled against canonical/session records. Thirteen QoS warnings, three AppIntents notices, cache and UI runtime observations remain disclosed; no timing-cause or warning-free claim is made. Fixture/hardware skips and four integration-class exclusions are not passes. The owner verified all 12,444 sealed evidence files and the affected composition review. All 14 live-base commits verify locally and on GitHub (
verified=true,reason=valid).This PR remains ready. Current-head hosted native inspection is recorded above. The composed native run does not establish a new authenticated cleanup-fault journey or fix terminal complete-WALLET availability. Existing authored QA IDs and limitations remain applicable. Evidence:
work/validation/ios1124-parent5a50-final-a/inspection/structured-report.jsonandwork/validation/ios1124-final-parent5a50-20261101/.Current parent composition — October 31
Signed additive merge
dd180c6c1bb5765cf43878f6bf04ef1d07b5a8a4incorporates live #1122/a6647db2614227a2b423c665701976034d995d7f; combined treec0b519ef61b2ce8c2c9aecb3de69585678028379. The complete 24-file child patch is unchanged after normalizing only blob IDs and hunk offsets. One fresh combined composition review is clean; prior unaffected security coverage is retained.The final build and selected native gate passed 2,968 unit +39 UI physical executions, with five explicit skips, zero failures and no retries (2,978 logical passes). All 1,692 frozen source hashes match. Complete raw logs, all 3,012 nested summaries, 3,071 referenced records and 67 attachments were inspected. The owner verified 18,632 sealed evidence files and 28 external inputs. SwiftFormat 0.64 is 0/828 with 108 exclusions; translations have zero errors and 15 specific warnings verified against the actual parent.
Diagnostics retain 12 canonical QoS issues/13 stdout emissions, 48 NaN messages and two AppIntents notices. The wrapper omitted three passing cases and all 12 QoS issues; canonical/raw/session records reconcile them. No timing-cause or pristine dependency-checkout claim is made: missing Git alternates still limit dependency working-tree certification. Native fixture, hardware and local-fork skips and four workflow integration exclusions are not passes.
All 12 live-base commits verify locally and on GitHub (
verified=true,reason=valid), with no history rewrite;draft=falseis verified. Current-head native CI and SwiftFormat are now fully audited: 2,968 unit +39 UI physical passes, five explicit skips, zero failures/retries, formatting 0/828 with 108 reported skips. Both attempt-1 jobs checked out syntheticb40808ea, whose tree exactly equals this published head against actual parenta6647db2. The artifact digest and all 6,175 ZIP members are verified; complete logs, all nested result references, canonical/session counts and 67 attachments were inspected. The owner verified 18,658 sealed evidence files and four external inputs. All 571 linker events/286 distinct messages match the sealed hosted actual parent; two AppIntents notices and other runtime/fixture warnings remain retained. No QoS or NaN marker was observed in this hosted run; that does not erase the local observations or establish a repair. Main Thread Checker injection is recorded, unit OS-log emission is disabled, and sanitizer coverage is not established. A real unsupportedLIGHTNING_CONNECTIONSbackup diagnostic remains disclosed and is not full-WALLET acceptance. Reader-cache and unavailable console-channel errors were retained; they did not rerun tests or CI. The original three-second route failure remains causally unproved, with its deadline/assertions unchanged. Full authenticated J1 cleanup/subscription/fault coverage and safe manual continuation remain distinct gaps; this composition is not a new authenticated journey pass.Historical parent composition — October 30, 22:47 UTC
Signed additive merge
de683f315d91aab493b17e5d41e1958e5ef5fd13incorporates #1122 at49f88a067d2c4947fd104329a4ae603719c23522. The original child patch is preserved. Final tree175bcc1c9feb8bc8b0c39fc7816c4b2d5e4ac6b7passed build-for-testing, 2,959 unit tests and five selected ProfileRecovery/ContactImport UI tests, with zero failures/retries. One opt-in encrypted native-recovery fixture test was skipped because its fixture URL was unset; the four workflow integration classes remain excluded. These are not additional runtime passes.SwiftFormat 0.64.0: 0/827 findings; translation validation: zero errors and 15 warnings matching the actual parent. All 1,685 frozen inputs match. Complete raw build/test output, nested results and diagnostics were inspected; five QoS observations, three AppIntents notices and simulator/UI runtime diagnostics remain retained. Warning-bearing test methods match the parent, without a parent runtime recurrence or causal claim. Two independent affected source reviews are clean. All eleven live-base range commits verify locally and on GitHub (
verified=true,reason=valid).That head’s native CI and SwiftFormat passed on the exact published tree: 2,959 unit and 39 physical UI passes, five explicit skips, zero failures or test reruns, formatting 0/827. Complete logs, every nested result, diagnostic payload and checksum/CRC-verified archive were inspected; the owner verified all 9,378 sealed evidence files. The 286 distinct linker messages match the actual parent; two AppIntents notices, one keyboard timeout and 48 NaN observations remain disclosed without a causal or harmlessness claim. Excluded integration classes and skipped migration/fixture cases are not passes.
This PR remains ready. The historical route-appearance failure is not explained by this passing selected run, and the remaining authenticated/manual cleanup cases retain their existing evidence limits. No deadline was weakened or unchanged CI retried. The earlier ENOSPC merge failure and intact pre-merge state remain recorded; disk recovery allowed this validated composition.
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 contact-removal acknowledgement, or fail profile deletion with a 404-like error without confirming the outcome. Also prepare active recurring subscriptions in both payer/payee roles, including one without a saved contact and an unrelated saved contact. Record endpoint/contact state, confirm removal, then end or wait for expiry of the subscription and explicitly confirm again. Include a fixed-term subscription for which cancellation is unavailable; verify truthful expiry guidance. 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.xmlbehavioral matrix completed by the author on currentb14d5a1bthrough normal authenticated app UI, including subscription refusal/ended-subscription progress, private/partial/unknown cleanup failure, 404, Cancel and cold restart. Exact configuration and limits are below; skipped component tests are not counted as runtime passes.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.PaykitSdkIdentityCheckTests.swiftand manager tests — both roles, orphan/non-Bitkit records, unknown/invalid/expired subscription ends, strict empty/nonempty batches, read failure/cancellation and early workflow refusal.Current-head authenticated acceptance — October 30
The author completed the frozen J1 behavioral matrix at
b14d5a1bthrough the normal app on a dedicated unfunded iPhone 16/iOS 26.2 (23C54) simulator, using real Homegate/Pubky grants and encrypted native storage:These manually driven normal-app cases satisfy the committed journey's required boundaries; no automated XML-runner execution is claimed. App pins remain Paykit RC2; separately validated companion helpers use RC3 source, not a published/adopted RC3. The signed configuration-only installed copy preserves audited unsigned executable/native bytes and normal startup/UI.
All failed preparations remain: blocked/linking preconditions, wrong-phase lost-ACK attempta (it hit later public cleanup after acknowledged profile deletion), and one sandbox/CoreSimulator launch failure recovered without reinstall/reset. Caseb's local-contact screenshot came after reconciliation, so a distinct narrow casec captured local retention while the fault was still active. Multiple rejected background reconciliation PUTs are disclosed (124/48); only one selected write was forwarded per case, with no profile DELETE. Reservation/outbound differences are not called immutable.
Complete runtime/structured diagnostics were inspected, including the25,283-line ended run,3,866-line lost-ACK run and3,248-line restart/success/local-retention capture. Injected storage errors, startup balance/RGS failures, temporary WALLET unavailability and post-sign-out Shop identityChanged remain disclosed; no clean-runtime or historical-cause claim. All faults are now accept, exact consumed fences remain, and native read-only observation found zero pending markers. No payment, live funds or wallet reset. The final disposable profile is preserved.
Evidence:
work/ios1124-authenticated-acceptance-final-20261030.md; exact case receipts/screens/native journals underwork/fixtures/ios1124-acceptance-20261029/{payer-refusal-a,orphan-refusal-a,partial-contact-b,unknown-contact-b,unknown-contact-c,final-success-a}/, plus the earlier payee/private/404 receipts. The prior112-entry seal remains byte-valid. Current source is unchanged, so matching final/native/format/signature evidence below is reused. Safe original-attempt Retry/Disconnect, Ring-owned cleanup, full-wallet redesign and cross-device contact CAS remain separate scopes.Evidence
Current signed additive head
b14d5a1b93ab274928fda044771d6e12c27d2475incorporates validated live #1122bee8975a9927d806beae82472cd76f2d63a49e49. All ten actual-base commits verify locally and asverified=true/reason=validon GitHub;draft=falseremains verified. The atomic subscription refusal and strict cleanup delta are preserved. Final combined-tree build/tests passed 2,883 units and five selected UI cases (four ContactImport, one ProfileRecovery), with one encrypted-native fixture skip, zero failures/retries. Other UI and four workflow integration classes were unselected. All 1,671 frozen hashes, 2,889 nested summaries and the 13,739-file audit seal match. SwiftFormat 0.64 reports 0/824, 100 configured exclusions; translations zero errors/15 warnings verified against the actual parent. Six Paykit initialization QoS observations and two AppIntents notices remain documented without a causal claim. One independent affected-delta review is clean, reusing the unchanged queue/subscription risk reviews. Current-head native CI and formatter passed on syntheticf814ed91, whose tree equals this head: 2,883 units and 39 physical UI passes, five prerequisite skips, zero failures or retries. All 2,927 nested summaries, eight complete diagnostic logs, 53 payloads and the checksum/CRC-verified archive were inspected; the owner verified 9,171 evidence hashes. Formatter reports 0/824 with 100 exclusions. All eight added test methods passed. The 286 distinct linker messages and two AppIntents notices match actual-parent sealed diagnostic metadata and producer inputs. Forty-eight NaN observations and one keyboard timeout in passing tests remain disclosed without a causal or harmlessness claim. Nine migration jobs were skipped; integration, external E2E and translation lanes do not run on this stacked target. The separate upstream failures/retries remain unwaived. The separately recorded current-head authenticated J1 matrix above is complete within its explicit fixture limits. Native/component CI remains distinct evidence. Affected human re-review is requested after evidence publication; no approval or merge is implied.Previous signed head
2df5bad1821193ceb1125536a1de428e583010d9incorporates live #1122940d7f3ec035bf27b6fa64d7b981a8188f1dc232and finite-expiry guidance6665a443. All nine actual-base commits verify locally and reportverified=true/reason=validon GitHub. The PR remains ready (draft=false). Current-head hosted native tests and SwiftFormat passed on the exact synthetic tree: 2,868 units plus 39 physical UI executions, zero failures/retries and five explicit skips (2,878 unique passes). Formatting reports 0/823. Both full logs, all 2,912 nested summaries, checksum/CRC-verified artifact and diagnostic files were inspected; the 11,944-file seal was verified. The 286 distinct linker warnings match the actual parent. No QoS observation was found; 72 CoreGraphics NaN notices remain in two passing USDT UI cases, with unproven cause. Nine migration jobs were skipped, not passed. These results do not execute the authenticated J1 matrix.Prior
2df5bad1native build/tests on iPhone 16 / iOS 26.2 (23C54) passed 2,868 unit tests plus 39 physical UI executions, zero failures or retries and five explicit fixture/hardware skips: 2,878 unique passes across 2,912 physical summaries. Four integration classes remain excluded. All 1,664 frozen source hashes match. Complete logs, nested XCResult issues and the 9,348-file evidence seal were inspected and verified. Twelve QoS observations, two AppIntents notices and 24 CoreGraphics NaN notices in the passing invalid-payment UI case remain disclosed; timing causes are unproven. SwiftFormat 0.64.0 reports 0/823, with 99 configured exclusions; translations report zero errors and 15 verified actual-parent warnings. The independent composition review is clean and preserves the prior risk-scoped review of the unchanged subscription guard. Three added expiry/unknown/cancellation controls passed. This is native component/UI evidence, not the authenticated J1 matrix.Previous
a2b21861hosted native tests and formatting passed on its exact synthetic tree: 2,832 units and 39 UI executions, five skips, no failures/retries, format 0/821. Its full artifact and diagnostics remain revision-scoped; no old-head pass is carried onto the current tree.The new subscription checks preserve the existing original-session validation and SDK operation lock. Unknown or malformed expiry remains active; a failed subscription read propagates its error. Generic bulk contact removal retains its existing behavior. Profile cleanup uses the strict batch, so a known active subscription cannot silently filter its contact and delete unrelated contacts. Final profile deletion retains the native expected-revision CAS. Cross-device atomicity across separate SDK calls is not claimed.
The controlled native before test failed for both payer and payee orphan subscriptions: the profile mutation ran when it should have refused (four assertion instances). The repaired focused SDK/manager/contact suites passed 251 cases with no failures/skips. The early workflow test verifies refusal before endpoint/contact cleanup and preserved session/sharing state. These tests inject native records and failures; they are not authenticated runtime acceptance.
Previous composition
c26d7b13, parentf3df6cda, passed 2,828 units and 39 UI executions locally, with three explicit Trezor skips. Hosted native tests and formatter passed on its exact tree: 2,828 units + 39 UI passes, five explicit fixture/hardware skips, no failures/retries, format0/821. Complete logs and checksum/CRC-verified nested artifacts were inspected. Duplicate-symbol and runtime diagnostics remain revision-scoped in the recorded evidence. These prior results do not validate the new subscription branch.Gaps
Current-head J1 is complete within the configuration and limits above. Local-only overrides were not separately altered in the author fixture; their unchanged controls and older attributed manual evidence retain their scope. Safe uncertain remote Retry/Disconnect, native cross-device contact CAS, Ring credential behavior and historical incident causation remain separate work and are not certified here. No human approval, app merge or RC3 adoption is inferred.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)Prior hosted failure
At
a0626189, run40165956252 failed the three-second pending-wallet route appearance assertion while later path/preparation assertions and 39 UI executions passed. Complete logs/nestedXCResult and the separate exact iOS 26.2 diagnostic pass are retained. Keyboard overlap did not establish causation. No deadline was weakened; later green compositions are not a causal repair claim.External authenticated review evidence — October26
Review5653598069 reports five controlled cases at
a0626189, using iPhone 17/SE simulators on iOS 26.5, Core 0.8.0-rc11 and Paykit 0.1.0. A debug fixture provisioned disposable native identities and displayed call/state evidence; the deletion workflow/SDK used a local homeserver proxy. Private-read503, accepted contact write with rejected shared-state publication, partial removal and GET200/DELETE404 preserved appropriate session/contact/sharing evidence and prevented later deletion. Cancel plus cold launch restored the same identity with no profile DELETE during the reported 60-second observation. Videos/traces are linked in that review.These are attributed external manual 1/2 results, not current-head author runs or proof of the added subscription cases, safe manual retry or native cross-device CAS. The earlier approval does not waive an unresolved finding or failed check.