Skip to content

fix: stop automatic profile deletion replay - #1688

Open
ben-kaufman wants to merge 2 commits into
fix/restore-identity-backgroundfrom
fix/profile-delete-session-recovery
Open

ben-kaufman wants to merge 2 commits into
fix/restore-identity-backgroundfrom
fix/profile-delete-session-recovery

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-ios#1122
Dependency: #1686

Description

  • Stop restarting the complete profile deletion flow after an error: earlier contact or profile mutations may already have succeeded when their acknowledgement is lost.
  • Remove the deletion-only session refresh helper while retaining normal initialization/recovery and the existing success/failure controls.

Out of Scope

  • Original-session ownership through Android cleanup and finalization, and safe reconciliation before manual retries: tracked in Bind profile deletion cleanup to the original sign-in #1687. This change removes automatic replay; it does not claim an identity-bound, resumable profile-deletion workflow.
  • Compatible native confirmed-401 request recovery remains dependent on fix: recover native grant requests after 401 pubky/pubky-homeserver#741 and subsequent Paykit artifact adoption. An invalid session may require normal recovery or re-authentication before an explicit deletion succeeds; broad transport errors do not justify replaying the destructive flow.

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

  • J1: new delete-profile-uncertain-response.xml — lost acknowledgement, honest failure feedback, no automatic deletion/sign-in replay, cancellation and restart without selecting Retry.

Automated Checks

  • Hosted final-head CI at 99b446a passed all 4,623 unit tests and all 39 existing E2E cases across seven shards. Complete build/lint/E2E logs inspected; the checksum-verified uploaded Detekt SARIF and Android Lint SARIF/XML have zero findings. These E2E cases do not execute J1.
  • Required just compile, just test and just lint passed. 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.
  • Two consecutive independent reviews found no actionable issues in this narrow change.

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

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)

@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Triage: 3.1.0. It fixes shipped profile deletion, and 3.1.0 is due 28 Oct.

@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This narrow change appears safe to merge; no actionable issues were found.

Summary

Removes automatic replay of profile deletion after an error. EditProfileViewModel now calls deleteProfile() directly, and the deletion-only session refresh helper is removed.

  • A failed profile deletion no longer starts an automatic second attempt.

Reviews (1) · Last reviewed commit: "fix: stop automatic profile deletion rep..." · Reviewed by Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 99b446a (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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; deleteAllContacts is unchanged by this diff.
  • The sign-out race test passing for the wrong reason: deletePaykitProfile is held on a deferred until signOut() returns, and the test asserts signIn is 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants