Repository navigation
fix: stop automatic profile deletion replay - #1688
ben-kaufman wants to merge 2 commits into
Conversation
|
Triage: 3.1.0. It fixes shipped profile deletion, and 3.1.0 is due 28 Oct. |
[Medium impact] This narrow change appears safe to merge; no actionable issues were found. SummaryRemoves automatic replay of profile deletion after an error.
Reviews (1) · Last reviewed commit: "fix: stop automatic profile deletion rep..." · Reviewed by Greptile |
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Removes the automatic session-refresh-and-redelete after a failed profile deletion, so EditProfileViewModel calls PubkyRepo.deleteProfile() once and an uncertain failure lands on the existing failure dialog. Reviewed 99b446a, full tier, as the delta on fix/restore-identity-background (stacked on #1686).
What I checked, and 3 candidates I ruled out
Read in full: PubkyRepo.deleteProfile and deleteAllContacts, EditProfileViewModel.attemptDeleteProfile, retryDeleteProfile and disconnectProfile, the rewritten PubkyRepoTest cases
Call sites traced: deleteProfileWithSessionRetry and refreshSessionIfPossible have no caller left under app/src/main (grep on the PR head)
CI: build (debug build and unit tests) and lint green; e2e shards still running when reviewed
Ruled out
- An expired session can no longer be refreshed inside deletion, so Retry keeps failing: deliberate and stated in Out of Scope; the failure dialog still offers Retry and Disconnect, and session restore through
initialize()is unchanged (the renamed contact-edit test now goes through it). - Retry re-runs contact cleanup: it did before too;
deleteAllContactsis unchanged by this diff. - The sign-out race test passing for the wrong reason:
deletePaykitProfileis held on a deferred untilsignOut()returns, and the test assertssignInis never called and the identity stays cleared.
Merge confidence: 5/5, build, unit tests and lint green, and PubkyRepoTest covers the single-delete path and the sign-out race.
Twin: synonymdev/bitkit-ios#1122
Dependency: #1686
Description
Out of Scope
Design
N/A — existing progress, failure options and navigation.
Preview
An uncertain deletion failure remains a failure instead of silently repeating contact cleanup and profile deletion.
QA Notes
Setup
Use committed released dependencies and synthetic identities for unit tests. J1 requires a disposable test identity and a homeserver fault proxy that accepts one profile deletion, drops its acknowledgement and retains a redacted request counter. That complete-app fixture is unavailable; do not use a personal profile, reset wallets or use live funds.
Journeys
delete-profile-uncertain-response.xml— lost acknowledgement, honest failure feedback, no automatic deletion/sign-in replay, cancellation and restart without selecting Retry.Automated Checks
just compile,just testandjust lintpassed. All 4,623 tests across 254 reports passed with zero failures/errors/skips. Complete Detekt SARIF and Android Lint XML/SARIF contain zero findings; all Gradle problem diagnostics match the unchanged base.Evidence and gaps
Android's pre-fix repository suite executed 223 tests with two expected counterexamples: a lost deletion response causes a second delete and false success, and the old refresh helper can authenticate an identity after sign-out. Removing the helper removes its sole production entry. The repaired 219-test repository suite passes, including one-delete/error preservation and held deletion failure after sign-out. The remaining manual deletion/finalization ownership issue is separate.
The iOS UI and helper call graph has the same automatic replay, and the patch removes that call path while preserving its existing ownership and failure guards. The full native suite is regression coverage; it is not an executed iOS view/proxy lost-acknowledgement journey. J1 remains unverified on both platforms. No historical contact-failure attribution is claimed.
Models used
gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh)